-
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 Kalle Alm as BIP editor #1116
Conversation
ACK 20bda62, assuming ACK by @kallewoof |
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.
ACK sans nit
bip-0002.mediawiki
Outdated
|
||
===BIP Editors=== | ||
|
||
The current BIP editor is Luke Dashjr who can be contacted at [[mailto:luke_bipeditor@dashjr.org|luke_bipeditor@dashjr.org]]. | ||
The current BIP editor are: |
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 BIP editor are: | |
The current BIP editors are: |
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.
Already proving yourself as an editor! Fixed.
20bda62
to
f5575fb
Compare
ACK f5575fb |
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.
ACK. The generalization of the document LGTM.
ACK |
Certainly no problem with @kallewoof being added as an additional editor. Only consideration re merging this is (imo) whether this change is included in the planned BIP 3 process revision that @kallewoof has planned and whether BIP 2 should be left as is given there will likely be a number of proposed changes for BIP 3 in addition to a new BIP editor. |
I don't understand why meta process BIPs need to be assigned a new BIP number each time they are modified. This just creates confusion as to which one is the latest active one. Also, it bloats the already large repo even more with historic documents that are irrelevant to the current process. Historians can always ask the git log for previous versions. |
@MarcoFalke: That's a question for @luke-jr. Previously he has said:
My only point is that a new BIP editor is unlikely to be the only proposed change to BIP 2. The Rejected rule also needs clarifying and there may be other proposed changes too. Either we attempt to make multiple changes to BIP 2 and ditch the idea of BIP 3 entirely. Or if we are making a BIP 3 then we should probably bundle those changes into BIP 3 and keep BIP 2 as is. |
There can only be one active meta process BIP. As soon as there is a new version, the previous version become inactive. The easiest way to achieve this is by simply editing the process BIP and merging the changes once they shall become active. Should we start to create a new BIP every time someone fixes a typo or changes a sentence? |
For reference: https://github.com/bitcoin/bips/wiki/BIP-Process-wishlist |
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.
ACK f5575fb
ACK f5575fb |
ACK adding @kallewoof as noted above. I am sympathetic to the idea that the process of becoming or un-becoming an editor should be defined 'n documented, but I think that @kallewoof and @luke-jr can propose something as opposed to the more aggressive plan/nack from @JaimeCaring. I think they shouldn't punt on doing so, however. |
ACK f5575fb |
A judgement for the current BIP editor (@luke-jr) and the future BIP editor (@kallewoof) on if/when it is appropriate for this to be merged. We have already had pressure for Taproot activation params to be merged (which they were), pressure for an additional BIP editor to be agreed to (which it was) and pressure for a new BIP editor to be chosen (which it was). At some point people need to step back and trust the individuals involved to move forward in their own time rather than pressuring for PR merges within a week of it being opened. Although ACKs from other reviewers are informative, in this repo ACKs generally only hold real weight re merge decisions if the ACKs are from BIP authors. @kallewoof: If you are uncomfortable with progress as you onboard as a new BIP editor in collaboration with Luke please raise it and we can discuss how to help you. Otherwise I'd ask people to leave them to it and consider the precedent they are setting by demanding merges within certain time periods. |
As I noted on IRC, I'm not uncomfortable at all, and I'm grateful to John for championing this as he's done so far. I have no issues waiting for Luke to get around to merging this, but I understand people's frustration. |
This is not true for meta BIPs, BIP 2 says:
|
Thanks for the correction. Then we need community consensus on potentially making multiple changes to BIP 2 or community consensus on what is included in a new BIP, BIP 3 as @kallewoof has tentatively put forward here. @kallewoof: What are your thoughts on making some of these proposed changes to the BIP process in BIP 2 versus starting afresh with a new BIP, i.e. BIP 3? edit: As I've said earlier merging this single PR into BIP 2 doesn't seem a problem to me. But if there are going to be multiple proposed changes to BIP 2 in addition to this PR we need to assess whether we need a new BIP 3 or not. I think @luke-jr would prefer a new BIP, BIP 3 for these changes and @MarcoFalke would prefer we just made changes to existing BIP 2. Presumably @jnewbery shares Marco's view. But other than those three people, I have no idea what anyone else thinks. Personally I think if there are going to be multiple proposed changes then it makes sense to start afresh with BIP 3. |
I fail to see how my quote implies that a change that has community consensus and is ready to be merged needs to be held back and bundled with changes that don't (yet) have community consensus. Also, the discussion seems slightly off-topic for this thread and a separate discussion might be more appropriate. |
Because once you make one change to BIP 2 you might as well make multiple. That effectively makes the BIP 2/3 decision.
I disagree it is off-topic for previous rationale. Though I agree a separate discussion is appropriate and something I expect to happen in future when Luke, Kalle are ready to have that discussion. Rushing through merges doesn't seem like a good idea to me generally, let alone when there are broader meta BIP issues to be resolved. |
I think updating BIP 2 to edit/add editors is a different question from modifying BIP 2, so I agree with @MarcoFalke here. Let's move the discussion of extensive/further changes to a separate thread (https://github.com/bitcoin/bips/wiki/BIP-Process-wishlist is a good starting point). |
ACK |
@luke-jr Seems ready for merge |
ACK f5575fb |
Concept ACK (I have not proof read the text yet) |
ACK commit f5575fb |
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.
ACK f5575fb
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.
ACK f5575fb
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.
Looks good for the most part.
A few nits:
Update language to clarify that there are multiple editors.
fda50a5
to
4e53b6e
Compare
@luke-jr - I've taken your review comments. This is ready for merge. |
ACK 4e53b6e corrects choice of singular editor to all editors in a few places |
ACK, thanks for the patience everyone. |
Update language to clarify that there are multiple editors.