Skip to content
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

Conversation

TheBlueMatt
Copy link
Contributor

As the BIP process restarts, one general risk in the BIPs is that they become (even more of) a dumping ground for half-baked ideas which even the BIP champion does not work on after the BIP is assigned. While simply never merging new BIPs addresses this issue, this isn't particularly sustainable, and we should instead have some concrete criteria here.

BIPs generally fall into a few categories with respect to when they should be considered ready for merge:

  • they're not particularly controversial (and not about consensus) and move forward towards adoption in several ecosystem projects relatively quickly or,
  • they generate lots of discussion, lots of feedback, the champion actively pushes the BIP forward and makes sustained arguments for adoption over some extended time period (this generally applies to consensus changes).

As long as the BIP champion is relatively actively pushing for adoption (i.e. active on the BIP outside of the BIP text itself) and actively addressing feedback on the BIP, the BIP should be merged, however if the BIP hasn't seen any adoption anywhere (outside of maybe one project the author wrote) and the BIP champion is off doing other things, I'd argue it clearly should simply be closed.

This commit adds some trivial but flexible text to that effect, requiring roughly the above criteria for merge.

I'd humbly suggest that we apply this criteria to new BIP number assignments (or at least BIP merges, even if they get a temporary number pre-merge) while we hash out whether this criteria makes sense.

As the BIP process restarts, one general risk in the BIPs is that
they become (even more of) a dumping ground for half-baked ideas
which even the BIP champion does not work on after the BIP is
assigned. While simply never merging new BIPs addresses this issue,
this isn't particularly sustainable, and we should instead have
some concrete criteria here.

BIPs generally fall into a few categories with respect to when they
should be considered ready for merge:
 * they're not particularly controversial (and not about consensus)
   and move forward towards adoption in several ecosystem projects
   relatively quickly or,
 * they generate lots of discussion, lots of feedback, the champion
   actively pushes the BIP forward and makes sustained arguments for
   adoption over some extended time period (this generally applies
   to consensus changes).

As long as the BIP champion is relatively actively pushing for
adoption (i.e. active on the BIP outside of the BIP text itself)
and actively addressing feedback on the BIP, the BIP should be
merged, however if the BIP hasn't seen any adoption anywhere
(outside of maybe one project the author wrote) and the BIP
champion is off doing other things, I'd argue it clearly should
simply be closed.

This commit adds some trivial but flexible text to that effect,
requiring roughly the above criteria for merge.
@kanzure
Copy link
Contributor

kanzure commented Apr 22, 2024

"Has to exist for a while" is a good improvement. It doesn't solve advanced persistent threats, but it is a modest improvement.

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

@@ -54,6 +54,7 @@ For a BIP 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.
BIP drafts should not be merged until the idea has materially progressed beyond the ideation phase - it should have multiple implementations from independent authors released in different software packages, and/or have received material public discussion feedback from diverse contributors with the BIP champion actively working towards adoption for an extended time period. While BIPs are author documents which must only meet certain minimum criteria, the BIP editors should strive to ensure there are not an unnecessary number of BIPs which never progress into broad implementation across the bitcoin ecosystem.
Copy link
Member

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.

Copy link
Member

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."

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@luke-jr
Copy link
Member

luke-jr commented Apr 22, 2024

Once there's multiple implementations, it's Final... seems like this would negate the Draft/Proposed stages?

@TheBlueMatt
Copy link
Contributor Author

In some cases, yes, but in the general case no as we also allow something to be "beyond the ideation phase" as long as the author has gotten material feedback and has been actively working on it for a while. Its really about avoiding things getting merged that had someone post a bip, got no feedback, then the author moved on to other things.

@ajtowns
Copy link
Contributor

ajtowns commented Apr 23, 2024

"it should have multiple implementations from independent authors released in different software packages" How are the multiple implementers meant to implement the same thing if they don't have a spec to reference?

Likewise, "the BIP champion actively working towards adoption for an extended time period" seems to put the cart before the horse: "you should adopt my proposal so that it can be assigned a BIP number".

OTOH, "have received material public discussion feedback from diverse contributors" is pretty reasonable at least. Could consider something along the lines of "proposals should not be assigned a BIP number if specification sections are incomplete or important details are absent making the proposal impossible or unnecessarily difficult to implement".

@TheBlueMatt
Copy link
Contributor Author

I believe you missed the or in the sentence :). The "have multiple implementations" thing is really just an escape valve such that if you have a few implementations already even if there wasn't much discussion (because it wasn't controversial) it can just be merged as a BIP. This was also explained in the PR description.

Likewise, "the BIP champion actively working towards adoption for an extended time period" seems to put the cart before the horse: "you should adopt my proposal so that it can be assigned a BIP number".

More generally, however, no one should be waiting to encourage people to adopt their protocol just because its waiting to be merged as a BIP?!

@ariard
Copy link

ariard commented Apr 25, 2024

I think this shall be proposed as a new BIP of type “process”. And not an edit of BIP2.

The “multiple implementation” makes only sense if it’s related to consensus / p2p / wallet / cryptography scheme etc.

@katesalazar
Copy link
Contributor

katesalazar commented Apr 25, 2024 via email

@murchandamus murchandamus added the Process Trying to update process or stuck due to disagreement about Process label May 9, 2024
Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not convinced that the BIP2-based Process should be changed to this degree at this stage, but would prefer that this is considered during work on an updated process.

@@ -54,6 +54,7 @@ For a BIP 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.
BIP drafts should not be merged until the idea has materially progressed beyond the ideation phase - it should have multiple implementations from independent authors released in different software packages, and/or have received material public discussion feedback from diverse contributors with the BIP champion actively working towards adoption for an extended time period. While BIPs are author documents which must only meet certain minimum criteria, the BIP editors should strive to ensure there are not an unnecessary number of BIPs which never progress into broad implementation across the bitcoin ecosystem.
Copy link
Contributor

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.

@TheBlueMatt
Copy link
Contributor Author

Makes sense. Closing as it can go towards BIP 3.

@TheBlueMatt TheBlueMatt closed this Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Process Trying to update process or stuck due to disagreement about Process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants