BIP3: Address additional review #1819
Conversation
murchandamus
added
Proposed BIP modification
Process
|
Happy with the changes. I'm fully behind BIP3. Good work, Murch👍 |
bip-0003.md
Outdated
| whether the original document should endorse a potential replacement BIP? Is it the original authors, the authors of the new | ||
| proposal, the BIP Editors, some sort of community process, or a mix of all of the above? | ||
| On the new BIP these problems don’t exist in the same manner. As it is freshly written, it is wholly owned by its | ||
| authors, neither is the community already invested, nor do the original BIP’s authors have a privileged role |
There was a problem hiding this comment.
"neither is" phrasing here is awkward IMO
There was a problem hiding this comment.
Changed to
As it is freshly written, it is wholly owned by its authors. The community is not yet invested and the original BIP’s authors do not have a privileged role in determining the content of the new BIP.
bip-0003.md
Outdated
| Deployed. | ||
| Complete is the final status for most successful Informational BIPs. | ||
|
|
||
| #### Deployed | ||
|
|
||
| A settled[^settled] BIP may be advanced to Deployed upon request by any community member with evidence[^evidence] that the idea | ||
| described in the BIP is in active use. Convincing evidence includes for example: an established project having deployed support | ||
| Deployed BIPs should not be changed. A Complete BIP should only be moved to Deployed once it is settled: after its |
There was a problem hiding this comment.
We may want to be clear what "changed" means here; perhaps something along the lines of:
s/not be changed/not be changed apart from bug fixes, other minor fixes or editorial-only changes/
There was a problem hiding this comment.
Dropped the first sentence and rewrote the last sentence.
|
What is the advantage of changing the diagram to a white background? That change makes it more disagreeable to read when using GitHub in dark mode; same for use in a presentation with a dark background (as I did a few days ago).
|
|
I think this "standard-track" footnote is confusing and could made clearer. Maybe we can do a call to discuss: https://github.com/bitcoin/bips/pull/1819/files#diff-0fe6969eba0422ddb0e7823d13092c03aa90122e0c5e66786c5d8b20f54719e6R625 |
bip-0003.md
Outdated
| Deployed. | ||
| Complete is the final status for most successful Informational BIPs. | ||
|
|
||
| #### Deployed | ||
|
|
||
| A settled[^settled] BIP may be advanced to Deployed upon request by any community member with evidence[^evidence] that the idea | ||
| described in the BIP is in active use. Convincing evidence includes for example: an established project having deployed support | ||
| Deployed BIPs should not be changed. A Complete BIP should only be moved to Deployed once it is settled: after its |
There was a problem hiding this comment.
Dropped the first sentence and rewrote the last sentence.
bip-0003.md
Outdated
| whether the original document should endorse a potential replacement BIP? Is it the original authors, the authors of the new | ||
| proposal, the BIP Editors, some sort of community process, or a mix of all of the above? | ||
| On the new BIP these problems don’t exist in the same manner. As it is freshly written, it is wholly owned by its | ||
| authors, neither is the community already invested, nor do the original BIP’s authors have a privileged role |
There was a problem hiding this comment.
Changed to
As it is freshly written, it is wholly owned by its authors. The community is not yet invested and the original BIP’s authors do not have a privileged role in determining the content of the new BIP.
There was a problem hiding this comment.
ACK 2ecef94 modulo a few comments that could be for a follow-up, along with #1819 (comment).
bip-0003.md
Outdated
| development emerges from the participation of stakeholders across the ecosystem. | ||
| The BIPs repository neither tracks community sentiment nor ecosystem adoption[^adoption] of BIPs beyond | ||
| the brief overview provided via the BIP status (see [Workflow](#workflow) below). | ||
| No formal or informal decision body governs Bitcoin development or decides acceptance[^acceptance] of BIPs. |
There was a problem hiding this comment.
Could "acceptance" be (mis)construed as acceptance into the BIPs repository? Perhaps "adoption" (or "community adoption") would be a better term.
| No formal or informal decision body governs Bitcoin development or decides acceptance[^acceptance] of BIPs. | |
| No formal or informal decision body governs Bitcoin development or decides adoption[^adoption] of BIPs. |
There was a problem hiding this comment.
The BIP editors do decide "acceptance" of BIPs in a similar way to which the IETF decides on publication of RFCs or internet drafts, though? Maybe better to use the terms "adoption" (when people actually implement a BIP) and "publication" (when a BIP is included in the bips repository), and drop this paragraph and usage of the word "acceptance" for BIPs?
There was a problem hiding this comment.
I like this suggestion.
There was a problem hiding this comment.
Thanks, that’s a good suggestion. I amended this section to use "publication" (when a BIP is merged) and "adoption" (when a BIP is implemented by projects). "Acceptance" now only appears in a footnote commenting on community sentiment.
bip-0003.md
Outdated
| @@ -159,7 +158,7 @@ appear in the following order. Headers marked with "\*" are optional. All other | |||
| * Version — The current version number of this BIP. See the [Changelog](#changelog) section below. | |||
| * Requires — A list of existing BIPs the new proposal depends on. If multiple BIPs | |||
| are required, they should be listed in one line separated by a comma and space (e.g., "1, 2"). | |||
| * Replaces — BIP authors may place the numbers of one or more prior BIPs in the Replaces header to recommend that their | |||
| * Replaces[^proposes-to-replace] — BIP authors may place the numbers of one or more prior BIPs in the Replaces header to recommend that their | |||
There was a problem hiding this comment.
| * Replaces[^proposes-to-replace] — BIP authors may place the numbers of one or more prior BIPs in the Replaces header to recommend that their | |
| * Replaces[^proposes-to-replace] — BIP authors may propose the numbers of one or more prior BIPs in the Replaces header to recommend that their |
There was a problem hiding this comment.
I think it’s okay the way it is. Usually, replacement of a prior BIP is a major part of the motivation for such a new BIP. The authors of the new BIP wholly own their Replaces header, they are putting forth their new proposal including the aspect that it replaces the prior BIP. If the relationship between the BIPs is portrayed incorrectly, it would be grounds to amend the header, not adopt the BIP, or reject the BIP outright, but as explained in the footnote, I don’t see an issue with the authors unilaterally expressing their intent here.
bip-0003.md
Outdated
| * License and License-Code — These headers list SPDX License Identifier(s) of the acceptable licenses under which the | ||
| BIP and corresponding code are available. See the [BIP Licensing](#bip-licensing) section below for a description of | ||
| the Licenses and their SPDX License Identifiers. If there are multiple acceptable licenses, each should be on a | ||
| separate line. |
There was a problem hiding this comment.
Ok, but then I suggest that the BIP3 draft is changed to use SPDX License Expressions instead. These can involve OR and this is much clearer than just listing them: It's a widely used standard, and anyone reading it will understand it without the need to read BIP3. It should be easy enough to change the existing BIPs.
(me in #1807 (comment))
There was a problem hiding this comment.
If that's to be done I think a separate PR that updates both the description/definition in BIP 3 and updates all the License: fields to comply with the new spec would be best. Not against it, but it seems like a lot of work that's not really all that valuable, so doesn't seem to me like it should be a blocker for anything else.
There was a problem hiding this comment.
Sorry, I must have misunderstood what you were proposing to refer to the license code representations.
Now that I am looking at it again, did you mean that we altogether drop our licensing scheme in favor of following the SPDX standard? If so, that sounds like a good idea, but I would like @ajtowns prefer that it were done as a separate pull request, possibly even while BIP 3 is proposed. In case I still don’t understand correctly, could you perhaps describe in a couple more sentences what you propose to do?
There was a problem hiding this comment.
did you mean that we altogether drop our licensing scheme in favor of following the SPDX standard?
Indeed!
If so, that sounds like a good idea, but I would like @ajtowns prefer that it were done as a separate pull request, possibly even while BIP 3 is proposed.
I agree, this should be done separately. Though the changes will be trivial, this will touch many existing BIPs.
There was a problem hiding this comment.
Would you be interested in working out with me what the change to the Licensing section should be, or perhaps just propose one yourself? If this change is adopted into BIP 3 before it is activated, the terms could be updated with the activation of BIP 3 along the other changes that I started to outline in #1820.
There was a problem hiding this comment.
Yep, I can open a PR once after this one here has been merged.
murchandamus
force-pushed
the
2025-03-bip3-followups
branch
from
e0fa15f to
f258ce9
Compare
bip-0003.md
Outdated
| @@ -159,7 +158,7 @@ appear in the following order. Headers marked with "\*" are optional. All other | |||
| * Version — The current version number of this BIP. See the [Changelog](#changelog) section below. | |||
| * Requires — A list of existing BIPs the new proposal depends on. If multiple BIPs | |||
| are required, they should be listed in one line separated by a comma and space (e.g., "1, 2"). | |||
| * Replaces — BIP authors may place the numbers of one or more prior BIPs in the Replaces header to recommend that their | |||
| * Replaces[^proposes-to-replace] — BIP authors may place the numbers of one or more prior BIPs in the Replaces header to recommend that their | |||
There was a problem hiding this comment.
I think it’s okay the way it is. Usually, replacement of a prior BIP is a major part of the motivation for such a new BIP. The authors of the new BIP wholly own their Replaces header, they are putting forth their new proposal including the aspect that it replaces the prior BIP. If the relationship between the BIPs is portrayed incorrectly, it would be grounds to amend the header, not adopt the BIP, or reject the BIP outright, but as explained in the footnote, I don’t see an issue with the authors unilaterally expressing their intent here.
bip-0003.md
Outdated
| development emerges from the participation of stakeholders across the ecosystem. | ||
| The BIPs repository neither tracks community sentiment nor ecosystem adoption[^adoption] of BIPs beyond | ||
| the brief overview provided via the BIP status (see [Workflow](#workflow) below). | ||
| No formal or informal decision body governs Bitcoin development or decides acceptance[^acceptance] of BIPs. |
There was a problem hiding this comment.
Thanks, that’s a good suggestion. I amended this section to use "publication" (when a BIP is merged) and "adoption" (when a BIP is implemented by projects). "Acceptance" now only appears in a footnote commenting on community sentiment.
bip-0003.md
Outdated
| * License and License-Code — These headers list SPDX License Identifier(s) of the acceptable licenses under which the | ||
| BIP and corresponding code are available. See the [BIP Licensing](#bip-licensing) section below for a description of | ||
| the Licenses and their SPDX License Identifiers. If there are multiple acceptable licenses, each should be on a | ||
| separate line. |
There was a problem hiding this comment.
Sorry, I must have misunderstood what you were proposing to refer to the license code representations.
Now that I am looking at it again, did you mean that we altogether drop our licensing scheme in favor of following the SPDX standard? If so, that sounds like a good idea, but I would like @ajtowns prefer that it were done as a separate pull request, possibly even while BIP 3 is proposed. In case I still don’t understand correctly, could you perhaps describe in a couple more sentences what you propose to do?
murchandamus
force-pushed
the
2025-03-bip3-followups
branch
2 times, most recently
from
5c6dbd1 to
9fc58e2
Compare
murchandamus
force-pushed
the
2025-03-bip3-followups
branch
from
9fc58e2 to
0138ff1
Compare
There was a problem hiding this comment.
Latest changes in https://github.com/bitcoin/bips/compare/9fc58e20..0138ff17 lgtm -- good improvements! Will re-review the full change.
murchandamus
force-pushed
the
2025-03-bip3-followups
branch
from
0138ff1 to
cd4fb49
Compare
|
Rebased on the latest |
|
Thanks for the review, @jonatack, @RubenSomsen @real-or-random, @ajtowns, and @darosior. Let’s see if there is more review until the end of this week, otherwise I think this is ready for merge. |
murchandamus
force-pushed
the
2025-03-bip3-followups
branch
from
cd4fb49 to
99b8541
Compare
BIP3 got a bit more review from @RubenSomsen, @darosior, and @jonatack. Thanks!
This PR addresses the review. Most of it related to minor phrasing improvements, typos, and added emphasis.
I also added mention of the Version header and Changelog to the Backward Compatibility section which I had overlooked, and explain why I consider the headers "Superseded-by" and "Replaces" asymmetric, and only propose a new name for "Superseded-by" but retain "Replaces".
This PR includes a deletion and readdition of the entire BIP 3, so as to provide further reviewers a surface to leave comments directly on the BIP text. These two commits will disappear without a trace when the PR is later squash-merged.