-
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
Require BIPs be "beyond the ideation phase" before being merged #1570
Closed
TheBlueMatt
wants to merge
1
commit into
bitcoin:master
from
TheBlueMatt:2024-04-not-a-dumping-ground
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back 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.
Agree with the premise. I interpret the added text as partially redundant with lines 34-46 above, but not completely, and it provides a clarifying counterweight to line 51 ("The BIP editors will not unreasonably reject a BIP"). As such, I think could be helpful to editors as a guideline.
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.
@TheBlueMatt Do you think this line in the repo README would need to be updated as part of this change?
"We are fairly liberal with approving BIPs, and try not to be too involved in decision making on behalf of the community."
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 think that line still makes sense - the editors are still not "making decisions" though I'd argue that the line in the readme may need to be stronger, bolt, and at the very top - even with this proposed change there will still be BIPs that are bad ideas and that people shouldn't implement, that that should be clear to anyone casually glancing at the repo.
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 sounds like a fine idea, but I’m not convinced by the approach. So far, implementation was considered a requirement for a BIP to move to Final, and the Draft status was used for a BIP that was not yet recommended to be implemented, or more generally still open to major changes. Under BIP2, number assignment had been happening when BIPs got merged, so in Draft status.
While I agree that it’s not useful to have a huge number of BIPs that never get adopted, it already takes significant effort to write up ideas sufficiently comprehensive to meet the "high quality" and "provides sufficient information to implement from the BIP" criteria.
If these two sentences were incorporated into our process, it would mean that we would only assign numbers to BIPs that are ready to progress to Final. This is a complete overhaul of the prior process and as such I would concur that this amendment rather be taken into account for a successor Process BIP instead of a change to BIP2.
Either way, it seems to me that a number assignment would be useful long before a BIP is ready to progress to Final.
Perhaps when we update our process, we could consider an approach where a BIP can be merged to the repository as a Draft before assigning a number, named "BIP-draft-author-topic", and then a number be assigned when a BIP progresses to Proposed.