-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add EIP: Versioning Scheme for EIPs #8034
Add EIP: Versioning Scheme for EIPs #8034
Conversation
✅ All reviewers have approved. |
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
The commit 564d13c (as a parent of 7a65704) contains errors. |
EIPS/eip-7577.md
Outdated
|
||
## Abstract | ||
|
||
This EIP introduces a versioning scheme for [Standards Track](./eip-1.md#eip-types) EIPs by applying semantic versioning based on changes made to the EIP's Specification section once its status has changed from `Draft` to `Review`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a good idea to provide more in depth information about semantic versioning for people who aren't familiar. You could do something similar to: https://eips.ethereum.org/EIPS/eip-5139#versioning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I didn't know I could add an entire external document as an asset. Added in 8319b2d.
EIPS/eip-7577.md
Outdated
|
||
1. A new version number that follows the semantic versioning scheme outlined above. | ||
2. The date when the changes where introduced. | ||
3. A link to the ethereum/EIPs PR that implements the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current rules forbid links to PRs. This isn't a formatting issue, so you'll have to talk to EIP editors if you want this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed link requirement in 2a0eb65.
I think a link to the changes would be helpful to readers - I'll follow up with EIP editors to gauge interest.
EIPS/eip-7577.md
Outdated
|
||
Tooling SHOULD be added to the ethereum/EIPs repository that checks that the CHANGELOG is updated and that the version number in the metadata header matches the latest version number in the CHANGELOG. | ||
|
||
EIPS that have dependencies on other EIPS SHOULD include the version of the dependency in the metadata "requires" field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please provide more information about how this would look. EIP should include enough detail to be implementable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Removed the following requirement, as it's impractical to update all downstream EIPs upon a version change:
EIPS that have dependencies on other EIPS SHOULD include the version of the dependency in the metadata "requires" field.
-
Modified the Specification with a suggestion for the mechanism to update an EIP version using repo tooling.
EIPS/eip-7577.md
Outdated
|
||
## Rationale | ||
|
||
A semantic versioning scheme enables clearer communication within the community about EIP status and allows the impact of a change to be ascertained at first glance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This belongs in the Motivation section.
The Motivation should be why is this proposal a good idea, where the Rationale should be why you made those specific decisions within the EIP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed from Rational in 0503e84.
EIPS/eip-7577.md
Outdated
|
||
A semantic versioning scheme enables clearer communication within the community about EIP status and allows the impact of a change to be ascertained at first glance. | ||
|
||
Making the version available in the EIP's metadata header additionally allows for programmatic parsing of the version number by tooling used in reference tests or by client teams. Currently, the ethereum/execution-spec-tests repository implements a rudimentary EIP version checker: EIP spec tests are required to declare the EIP's markdown file digest SHA that the test implementation was based on. The current value of the digest SHA is then polled via the Github API to verify that no changes have occurred since the test implementation. While this provides a warning to test implementers that the EIP has changed, it is clearly of limited use. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid external links (ethereum/execution-spec-tests).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in 92fb8ba. The repository is still referred to, I hope that's ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fyi links to execution-spec-tests were recently allowed!
Additionally remove requirement that dependent eips must include the version of the dependency.
Co-authored-by: Nikki Gaudreau <87712502+gaudren@users.noreply.github.com>
…IPs into versioning-scheme-for-eips
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
3. The ethereum/EIPs PR number that implements the changes. | ||
4. A line for each change made to the EIP's specifications that includes a short description of the change. | ||
|
||
Additionally, the new version MUST be added to the metadata header of the EIP's markdown file (to a new "version" field), so that it may be easily parsed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to a new "version" field
This should have higher billing. Like the first line after the RFCs. As well as the new changelog section.
A new header field `version` is added to new EIPs, as defined below, identifying the draft and/or review version of the specification.
A new body section "Changelog" is added to new EIPs, as defined below, tracking the changes between draft and/or review versions of the specificaiton.
Also, we should update the EIP template to add these two new feaures as part of this EIP when it goes final.
|
||
## Abstract | ||
|
||
This EIP introduces a versioning scheme for [Standards Track](./eip-1.md#eip-types) EIPs by applying [Semantic Versioning 2.0.0](../assets/eip-7577/semver.md) based on changes made to the EIP's Specification section once its status has changed from `Draft` to `Review`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This EIP introduces a versioning scheme for [Standards Track](./eip-1.md#eip-types) EIPs by applying [Semantic Versioning 2.0.0](../assets/eip-7577/semver.md) based on changes made to the EIP's Specification section once its status has changed from `Draft` to `Review`. | |
This EIP introduces a versioning scheme for [Standards Track](./eip-1.md#eip-types) within an EIP by applying [Semantic Versioning 2.0.0](../assets/eip-7577/semver.md) based on changes made to the EIP's Specification section once its status has changed from `Draft` to `Review`. |
To my best understanding this proposal is about versioning within EIP instead of across EIPs, right?
author: danceratopz (@danceratopz), Ahmad Bitar (@smartprogrammer93) | ||
discussions-to: https://ethereum-magicians.org/t/add-eip-versioning-scheme-for-eips/17295 | ||
status: Draft | ||
type: Meta |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this shall be Informational
or Meta
. I like the idea and direction. I don't think every EIP needs adopt it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Editorially I think this is good enough to merge as a Draft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All Reviewers Have Approved; Performing Automatic Merge...
This PR proposes a versioning scheme for Standards Track EIPs based on their Specifications section.
An extended rationale can be found at https://notes.ethereum.org/@danceratopz/eip-versioning.
Discussion: https://ethereum-magicians.org/t/add-eip-versioning-scheme-for-eips/17295.