Add new two-week review process to EIPs #1100
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ eip: 1 | |
| title: EIP Purpose and Guidelines | ||
| status: Active | ||
| type: Meta | ||
| author: Martin Becze <mb@ethereum.org>, Hudson Jameson <hudson@ethereum.org>, [and others](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md). | ||
| created: 2015-10-27, 2017-02-01 | ||
| --- | ||
|
|
||
|
|
@@ -31,59 +31,49 @@ There are three types of EIP: | |
|
|
||
| ## EIP Work Flow | ||
|
|
||
| The normal process is: | ||
|
|
||
| ``` | ||
| [ A pull request ] -> [ DRAFT ] -> [ LAST CALL ] -> [APPROVED] -> [ FINAL ] | ||
|
There was a problem hiding this comment. I'm still concerned this oversimplifies matters, especially since this only describes the process for non-core EIPs - the process for core EIPs is different. Also, 'approved' isn't part of the non-core process; it goes straight from last call to final. It's more like: Core EIPs: Draft -> Editor last call -> Approved(?) -> Accepted -> Final Non-core EIPs: Draft -> Last call -> Final There was a problem hiding this comment. My understanding is that Last Call will cause a core EIP to go from this EIP process to the core dev meeting, where upon the best outcome is that it is approved. Then at some point it is deployed to a majority of clients. The fact that EF deploys it to EF-supported clients seems immaterial from our EIP perspective. The time when 51% of clients are running it seems relevant from our perspective. I am not trying to change things here or rock the boat. I am honestly trying to get Last Call in, which of course touches into these other processes. Please help me understand how to fix this and get it done. Also, please give me a call so we can figure this out together. I am not proposing this now, but another option is: Where non-core has EIP editors making "final" decision. And Core has Core Devs making "final" decision. Then "deployed" is merely a review of 51% deployment (adjudicated by EIP editors). This is how I understand the process as actually happening. There was a problem hiding this comment.
The process for Core EIPs has always involved back-and-forward between the EIP process and core devs. Progressing to the 'Accepted' and 'Final' states require approval from All Core Devs, with 'Final' requiring that it's been implemented for a future hard fork. I don't see anything in this PR that changes that, so it's important that any proposed diagrams adequately describe the actual process. There was a problem hiding this comment. I am using "accepted as draft", clearly that wording doesn't make sense if "accepted" is a status too. I thought the core devs only made the decision to implement. I thought the users decided to upgrade and the EIP editors to recognize when that upgrade was deployed. It is not clear to me how to update my wording to fix it and get this accepted. It is also not clear to me how the process actually works or how it is written on paper. This is why I asked for a phone call. I am documenting these things and I can make it great I just need to understand it first for myself. There was a problem hiding this comment. One issue is your note "closing PRs as soon as possible by either merging or rejecting them" -- this does not match the diagram. I want to understand this fully but all the questions and answers here are piecemeal. If I understand it completely I can write it well. There was a problem hiding this comment.
The status you want is just 'Draft', not 'Accepted as Draft'.
For Core EIPs, the devs decide to implement, and when to configure a hard fork including the changes. If that hard fork is successful, then the state changes to Final.
I already provided a suggestion for an alternate wording; what was wrong with that?
The diagram shows the states that EIPs can be in, not the process for updating them. A PR is how you update the state or contents of an EIP. |
||
| ``` | ||
|
|
||
| The [EIP editors](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md#eip-editors) update the status of EIPs at the request of EIP authors. Please ping the editors by mentioning them in your pull request or send them an email. See alse [EIP Editor Responsibilities & Workflow](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md#eip-editor-responsibilities-and-workflow). | ||
|
There was a problem hiding this comment. Corrected in 03aab50 |
||
|
|
||
| The EIP process begins with a new idea for Ethereum. It is highly recommended that a single EIP contain a single key proposal or new idea. The more focused the EIP, the more successful it tends to be. A change to one client doesn't require an EIP; a change that affects multiple clients, or defines a standard for multiple apps to use, does. The EIP editor reserves the right to reject EIP proposals if they appear too unfocused or too broad. If in doubt, split your EIP into several well-focused ones. | ||
|
|
||
| Each EIP must have a champion—someone who writes the EIP using the style and format described below, shepherds the discussions in the appropriate forums, and attempts to build community consensus around the idea. | ||
|
|
||
| Vetting an idea publicly before going as far as writing an EIP is meant to save the potential author time. Ask the Ethereum community first if an idea is original to avoid wasting time on something that will be be rejected based on prior research (searching the Internet does not always do the trick). It also helps to make sure the idea is applicable to the entire community and not just the author. Just because an idea sounds good to the author does not mean it will work for most people in most areas where Ethereum is used. Examples of appropriate public forums to gauge interest around your EIP include [the Ethereum subreddit], [the Issues section of this repository], and [one of the Ethereum Gitter chat rooms]. In particular, [the Issues section of this repository] is an excellent place to discuss your proposal with the community and start creating more formalized language around your EIP. | ||
|
|
||
| Once the champion has asked the Ethereum community whether an idea has any chance of acceptance a draft EIP should be presented as a [pull request]. | ||
|
|
||
|
There was a problem hiding this comment.
Not sure why this is removed. There was a problem hiding this comment. Previously draft process was more difficult, now we have automerge bot. This means that waiting to merge to draft is no longer a reason to delay drafts. There was a problem hiding this comment. The automerge bot is used for editing existing draft. But to merge a PR as a new draft, an editor still needs to assign the EIP number before it can be automerged. There was a problem hiding this comment. Thank you for clarifying. I have added this back in with d09dd90. This commit clarifies that the author requests merging. But I have reworded slightly to remove "after they have finished editing it" because they are not finished editing it. @jamesray1 If you are more comfortable with another wording, please feel free to directly add a commit into this PR. |
||
| EIP authors can request for the pull request to be merged as Draft when significant progress is made. Remember, Drafts can always be edited after merging. If the EIP editors accept the EIP as a draft ([see the EIP editors workflow section below for details](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md#eip-editor-responsibilities-and-workflow)), the EIP editor will assign the EIP a number (generally the issue or PR number related to the EIP) and merge your pull request. The EIP editor will not unreasonably deny an EIP. Reasons for denying EIP status include duplication of effort, being technically unsound, not providing proper motivation or addressing backwards compatibility, or not in keeping with the [Ethereum philosophy](https://github.com/ethereum/wiki/wiki/White-Paper#philosophy). | ||
|
There was a problem hiding this comment. The intended workflow is for PRs to be promptly either accepted, rejected, or have alterations requested. I'd rather not normalise a process where people open 'WIP' PRs and leave them that way indefinitely. There was a problem hiding this comment. Currently is seems that the default action is getting to draft and keeping it open. We should change that. But I argue that this text above is accurate. Please let's merge this and then correct the process with a separate PR. There was a problem hiding this comment. We are changing that, and I've already moved to closing PRs as soon as possible by either merging or rejecting them. Enshrining the previous process in EIP 1 would be a step backwards. |
||
|
|
||
| Once the first draft has been merged, you may submit follow-up pull requests with further changes to your draft until such point as you believe the EIP to be mature and ready to proceed to the next phase. | ||
|
|
||
|
There was a problem hiding this comment. Again, I don't see why this should be removed. There was a problem hiding this comment. I have re-added this sentence with 58d22b4. I have changed to "accepted as draft" to clarify which status this is. |
||
| Standards Track EIPs consist of three parts, a design document, implementation, and finally if warranted an update to the [formal specification]. The EIP should be reviewed and accepted as Draft before an implementation is begun, unless an implementation will aid people in studying the EIP. Standards Track Core EIPs must be implemented in at least three viable Ethereum clients before it can be considered Final. | ||
|
|
||
| For an EIP to be accepted it must meet certain minimum criteria. It must be a clear and complete description of the proposed enhancement. The enhancement must represent a net improvement. The proposed implementation, if applicable, must be solid and must not complicate the protocol unduly. | ||
|
|
||
| An EIP in draft status must have implementations to be considered for promotion to the next status. When no further changes are planned and adove rules are met, the champion will request, and the editor will approve a change to "Last Call" status. This EIP will listed prominently on the http://eips.ethereum.org/ website (subscribe via RSS here [TODO LATER, ADD A LINK HERE]). After the review period, if all issues are resolved, then the editor will proceed to the next status; if issues remain then it may be returned to draft. | ||
|
There was a problem hiding this comment. Please put any TODOs in comments so they won't show up for readers. There was a problem hiding this comment. RSS feed coming in a sec... There was a problem hiding this comment. RSS feed added with d0735df |
||
|
|
||
| Standards track EIPs of type "Core" that pass "Last Call" will be sent to the core devs meeting for review. A successful review will lead to "Accepted" status. And when implemented in at least two clients totalling at least 50% of deployed nodes, and all pass a common set of test suites, the status of the EIP is updated to "Final". Alternatively, non-core EIPS will proceed directly from "Last Call" to "Final". | ||
|
|
||
|
There was a problem hiding this comment. I just noticed you're still removing this - why? |
||
| Other exceptional outcomes for EIPS include "deferred", a temporary status for drafts which have no recent progress, and "rejected" which records an EIP that is fundamentally broken and will not be implemented. Also, "active" denotes an EIP that is in effect but which may be updated without changing its EIP number. A "final" EIP should be updated on to correct errata, although another EIP may wish to "supersede" it — in that case you would have an original and succesor EIP both in final status. | ||
|
There was a problem hiding this comment. 'Deferred' is for core EIPs that have been put off for a future hard fork. There was a problem hiding this comment. Thank you, updated in 50ae6ac |
||
|
|
||
| ## What belongs in a successful EIP? | ||
|
|
||
| Each EIP should have the following parts: | ||
|
|
||
| - Preamble - RFC 822 style headers containing metadata about the EIP, including the EIP number, a short descriptive title (limited to a maximum of 44 characters), and the author details. See [below](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md#eip-header-preamble) for details. | ||
| - Simple Summary - “If you can’t explain it simply, you don’t understand it well enough.” Provide a simplified and layman-accessible explanation of the EIP. | ||
| - Abstract - a short (~200 word) description of the technical issue being addressed. | ||
| - Motivation (*optional) - The motivation is critical for EIPs that want to change the Ethereum protocol. It should clearly explain why the existing protocol specification is inadequate to address the problem that the EIP solves. EIP submissions without sufficient motivation may be rejected outright. | ||
| - Specification - The technical specification should describe the syntax and semantics of any new feature. The specification should be detailed enough to allow competing, interoperable implementations for any of the current Ethereum platforms (cpp-ethereum, go-ethereum, parity, ethereumJ, ethereumjs-lib, [and others](https://github.com/ethereum/wiki/wiki/Clients). | ||
| - Rationale - The rationale fleshes out the specification by describing what motivated the design and why particular design decisions were made. It should describe alternate designs that were considered and related work, e.g. how the feature is supported in other languages. The rationale may also provide evidence of consensus within the community, and should discuss important objections or concerns raised during discussion. | ||
| - Backwards Compatibility - All EIPs that introduce backwards incompatibilities must include a section describing these incompatibilities and their severity. The EIP must explain how the author proposes to deal with these incompatibilities. EIP submissions without a sufficient backwards compatibility treatise may be rejected outright. | ||
| - Test Cases - Test cases for an implementation are mandatory for EIPs that are affecting consensus changes. Other EIPs can choose to include links to test cases if applicable. | ||
| - Implementations - The implementations must be completed before any EIP is given status “Final”, but it need not be completed before the EIP is accepted. While there is merit to the approach of reaching consensus on the specification and rationale before writing code, the principle of “rough consensus and running code” is still useful when it comes to resolving many discussions of API details. | ||
| - Copyright Waiver - All EIPs must be in the public domain. See the bottom of this EIP for an example copyright waiver. | ||
|
|
||
| ## EIP Formats and Templates | ||
|
|
@@ -103,9 +93,11 @@ Each EIP must begin with an RFC 822 style header preamble, preceded and followed | |
|
|
||
| ` * discussions-to:` <url> | ||
|
|
||
| ` status:` <Draft | Last Call | Approved | Final | Active | Deferred | Rejected | Superseded> | ||
|
There was a problem hiding this comment. Why the rename from 'Accepted' to 'Approved'? There was a problem hiding this comment. The word "accepted" had previously been used many times with different meaning in the text so as to make the word ambiguous. I assume you prefer not changing Accepted based on this comment. So I have reworded everything to preserve "Accepted" as the state name. Other uses of the word are now "agreeable", "supported" etc. to avoid using "accept" for other meanings. Updated in cef7d9d |
||
|
|
||
| `* review-period:` Starting YYYY-MM-DD, ending YYYY-MM-DD | ||
|
There was a problem hiding this comment. This should be just a single date - either start or end - in order to be human-readable. There was a problem hiding this comment. Updated in 0ebde39 |
||
|
|
||
| ` type: `<Standards Track (Core, Networking, Interface, ERC) | Informational | Meta> | ||
|
|
||
| ` * category:` <Core | Networking | Interface | ERC> | ||
|
|
||
|
|
||
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.
Unfortunately I think this will break jekyll's postprocessing. If you want to keep it, can you check by running a local copy?
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.
Doesn't render nicely. But it is not broken. Respectfully requesting to merge as is and improve this rendering later.
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 agree that this is out of scope for this PR, but it would be nice to fix this rendering. Here is one option: https://stackoverflow.com/questions/28819413/add-link-to-yaml-in-multimarkdown-metadata.
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.
Thank you for the link. P.S. presently your review status on this PR is denied. Is anything else outstanding to earn your approved vote?
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.
As a quick temporary fix, perhaps we could change this to:
..., and others:
https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1.md
This would at least make it easier to copy and paste the link. I'm not sure how this renders, e.g. if newlines will work.
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.
@jamesray1 Does this resolve all your issues? Will you please mark PR as approved.
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.
Great, but the change isn't showing above and that commit hash isn't in the history.
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.
Oh yeah the extra colon would have stuffed it up. If you put the whole value in quotes that should work, let me try that locally.
https://stackoverflow.com/questions/8783705/how-could-should-i-state-colon-punctuation-in-a-yaml-file
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'm having an issue with jekyll, which is unrelated to the change in the author filed, so I can't test this.
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.
Sorry about that, this time the push went through. Please repull.