Update README with versioning information#704
Conversation
|
/cc @fjl, @lightclient, @LukaszRozmej, @macfarla and any other client devs I missed, would be great to get your feedback here too! |
|
@kclowes take a look at EIP for versioning EIP's, maybe you will have some inspiration: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7577.md |
danceratopz
left a comment
There was a problem hiding this comment.
Good idea. I think adding/removing might be the wrong way around though? Adding something doesn't break anything for users, whereas removing it does.
README.md
Outdated
| Examples of changes by version type: | ||
|
|
||
| **Major:** | ||
| - Adding new RPC methods |
There was a problem hiding this comment.
| - Adding new RPC methods | |
| - Removing an existing RPC method |
There was a problem hiding this comment.
you could clarify it by saying "Adding tests for a new RPC method"
README.md
Outdated
| - Adding new fields to response objects | ||
|
|
||
| **Minor:** | ||
| - Removing an existing RPC method |
There was a problem hiding this comment.
| - Removing an existing RPC method | |
| - Adding a new RPC method |
There was a problem hiding this comment.
I think both adding and removing should be in major. Here we are talking about what breaks hive tests not what breaks user experience. As such, both adding and removing is a major break in hive
There was a problem hiding this comment.
Yeah, exactly. As I'm optimizing for client developer experience here, adding a new RPC method will start clients failing on hive and so I think it should be a major. I can see either way on removing, but had it in minor because it won't change whether or not clients pass the hive tests.
There was a problem hiding this comment.
you could clarify it by saying "Removing tests for an existing RPC method"
README.md
Outdated
| - Adding new fields to response objects | ||
|
|
||
| **Minor:** | ||
| - Removing an existing RPC method |
There was a problem hiding this comment.
you could clarify it by saying "Removing tests for an existing RPC method"
README.md
Outdated
| Examples of changes by version type: | ||
|
|
||
| **Major:** | ||
| - Adding new RPC methods |
There was a problem hiding this comment.
you could clarify it by saying "Adding tests for a new RPC method"
README.md
Outdated
| **Patch:** | ||
| - Fixing typos or improving clarity in documentation | ||
| - Correcting examples in the specification | ||
| - Fixing test cases that don't match the spec |
There was a problem hiding this comment.
this one maybe should be minor? if clients are matching the tests rather than the spec (which is not uncommon), this will break things
|
Updated per the comments! @fjl if you feel like this is good to go in, I can tag and then open a PR against hive to pick up the tag. Or, let me know if there is anything else that should be adjusted. |
|
It's good to have this change in, and @jshufro will work on updating the release process to add the versioning. |
The idea with this versioning scheme is to minimize breaking cycles for client developers. As it stands, any new PR merged has the potential to cause failures in hive's rpc-compat test runs. I think it would be good to have a hive rpc-compat run against main with the latest changes, but to also have another rpc-compat hive run that runs against a stable version of this repo so we can give clients time to upgrade without flagging new changes as failures right away. Since the current version is 1.0.0-beta.4 I propose we tag what's currently in main as v1.0.0 and then we can start merging things like #678.