📅 Original date posted:2023-04-20
🗒️ Summary of this message: Public communication and explanations from reviewers and maintainers can improve the transparency and scalability of the open source project. Lack of communication can lead to confusion and difficulty for new contributors. The departure of long-time maintainers may also contribute to delays in merging pull requests.
📝 Original message:Thanks for this Andrew.
> What commentary does there need to be?
There doesn't "need" to be explanations about anything. There doesn't "need" to be any review comments whatsoever from anybody. But a world where reviewers explain what they've done to satisfy themselves that a pull request is ready to merge and a world where maintainers explain their thought process behind a merge decision is much easier to follow and much more scalable than the current black box where people see pull requests being self merged by a maintainer with no ACKs within a day of it being opened. Most likely these decisions make sense (low risk, unlikely to be reviewed by anybody else, blocking other pull requests etc). But more and more people are funded to work on Core and increasingly they seem to stick to their own mini projects and not review anybody else's work. Of course you can't put the responsibility for this entirely down to maintainers but the black box isn't scalable. Maintainers (presumably) have private discussions and so know how best to spend their review time. Everyone else (especially new contributors) are playing an uninformed and in the dark lottery with how they spend their review time (to the extent that they allocate any).
> There are currently 320 open PRs and 366 open issues.
I wake up every morning to 150+ email notifications containing
everything that went on overnight, and throughout the day, I typically
get hundreds more.
Indeed. All the more reason for more and higher quality public communication. If you struggle and you're in those private discussions with other maintainers on merge decisions and ready for merge discussions how do you think everyone is trying to assess how to spend their time? It is even more bewildering. As far as I know most of the current maintainers haven't worked on other projects or in the private sector for a sustained period of time but the way other projects and businesses function is that those with the most experience and most responsibilities are able to manage and provide guidance to those with less experience and less responsibilities. I'm sure this goes on within Brink if you've been anointed but this is supposed to be an open source project. If everything is done in private conversations and everything other than the code is open source it is pretty much a façade. It is very hard to make meaningful contributions without getting anointed. Those who do get anointed very early in their careers seem especially bad at hoarding information, refusing to do anything in public and not assisting those who haven't been anointed.
> Things can simply fall through the cracks.
> With several long time maintainers stepping away, this may be a factor
in PRs taking longer to get merged as the remaining maintainers may be
less familiar with the parts of the codebase that were previously
maintained by someone else.
> Requiring maintainers to have to write explanations for every single
merge is simply going to increase the burden on them and increase the
rate of burnout and resignations.
> We've had too many maintainers step down already.
This all points to a a need for additional maintainers (assuming they are sufficiently competent and qualified). We had a potential maintainer (Vasil) come forward (long term contributor, made significant contributions over a number of years, a qualified reviewer, contributes to a part of the codebase that current maintainers aren't very familiar with, ACKed by maintainers and long term contributors) and it was blocked. How does that make sense? You seem to want it both ways. We can't ask maintainers to meet higher standards because there's a shortage and they're close to burning out. But there's no need to add a new maintainer.
I've responded to the parts I disagree with and see inconsistencies with but generally I thought it was a very thoughtful and informative response so thank you. Of the current maintainers you seem to me to be one of (if not the) most responsive and open to public discussion on the project. I've learnt a tonne from your StackExchange posts and Twitch streams that are all public/open source that you do in addition to your contributions and maintenance of Core.
Thanks
Michael
--
Michael Folkson
Email: michaelfolkson at protonmail.com
Keybase: michaelfolkson
PGP: 43ED C999 9F85 1D40 EAF4 9835 92D6 0159 214C FEE3
------- Original Message -------
On Wednesday, April 19th, 2023 at 22:33, Andrew Chow via bitcoin-dev <bitcoin-dev at lists.linuxfoundation.org> wrote:
> Responses in-line.
> Note that the opinions expressed in this email are my own and are not
> representative of what other maintainers think or believe.
>
> On 04/18/2023 08:40 AM, Michael Folkson via bitcoin-dev wrote:
>
> > Communication has been a challenge on Bitcoin Core for what I can
>
> tell the entire history of the project. Maintainers merge a pull request
> and provide no commentary on why they’ve merged it.
>
> What commentary does there need to be?
> It's self evident that the maintainer believes the code is ready to be
> merged, and has observed enough ACKs from contributors that they are
> comfortable to do so.
> You're welcome to ask for clarification, but frankly, I don't think
> having any commentary on merges is going to be helpful or more elaborate
> in any way.
> Requiring maintainers to have to write explanations for every single
> merge is simply going to increase the burden on them and increase the
> rate of burnout and resignations.
> We've had too many maintainers step down already.
> It'll end up being a bunch of boilerplate comments that don't say
> anything meaningful.
>
> There are certainly situations where PRs are merged very quickly or with
> otherwise little apparent review.
> But, as I said, if you ask a maintainer why it was merged, the answer
> will be "I thought it was ready and had enough review".
> There may be other reasons that made the maintainer think it was ready
> sooner, such as the PR fixes a critical bug or security vulnerability,
> but these reasons aren't going to be stated publicly.
>
> > Maintainers leave a pull request with many ACKs and few (if any)
>
> NACKs for months and provide no commentary on why they haven't merged it.
>
> There are currently 320 open PRs and 366 open issues.
> I wake up every morning to 150+ email notifications containing
> everything that went on overnight, and throughout the day, I typically
> get hundreds more.
> It's impossible to keep up with everything that goes on throughout the repo.
> ACKs come in sporadically, PRs are updated, reviews are posted, etc.
> Often times PRs are not merged simply because the maintainers were not
> aware that a PR was ready to be merged.
> Things can simply fall through the cracks.
>
> Of course there are other reasons why something might not be merged, and
> these generally fall into the camp of "I don't think it has had enough
> review".
> It's the maintainer's judgement call to make as to whether something has
> been sufficiently reviewed, and part of the judgement call is to
> consider the quality and competence of the reviewers.
> If a PR had 100 ACKs but all from random people who have never
> contributed to the project in any capacity, then it's not going to be
> merged because those reviewers would be considered low quality.
> It's not just about the numbers, but also about whether the reviewers
> are people the maintainers think are familiar enough with an area and
> have had a history of thoroughly reviewing PRs.
> For example, if a reviewer who primarily works on the mempool reviewed a
> PR in the wallet, I would consider their review and ACK with less weight
> because they are unlikely to be familiar with the intricacies of the wallet.
> Obviously that changes over time as they make more reviews.
> For another example, if I see an ACK from a reviewer who posts reviews
> that primarily contain nits on code style and other trivialities, I
> would consider that ACK with less weight.
>
> Furthermore, the maintainers are not necessarily the ones who block a merge.
> Part of evaluating if something is ready to be merged is to read the
> comments on a PR.
> Other frequent contributors may have commented or asked questions that
> haven't been resolved yet.
> PRs will often not be merged (even if they have ACKs) until a maintainer
> deems that those comments and questions have been sufficiently resolved,
> typically with the commenter stating in some way that their concerns
> were addressed.
> In these situations, no commentary from maintainers is given nor
> necessary as it should be self evident (by reading the comments) that
> something is controversial.
> These kinds of comments are not explicit NACKs (so someone who is only
> counting (N)ACKs won't see them), but are blocking nonetheless.
>
> Lastly, personally I like to review every PR before I merge it.
> This often means that a PR that might otherwise be ready to be merged
> wouldn't be merged by myself as I may not be familiar with that part of
> the codebase.
> It may also mean that I would require more or specific additional people
> to review a PR before I merge it as I would weight my own review less
> heavily.
> With several long time maintainers stepping away, this may be a factor
> in PRs taking longer to get merged as the remaining maintainers may be
> less familiar with the parts of the codebase that were previously
> maintained by someone else.
>
> > but a casual observer would have only seen Concept ACKs and ACKs with
>
> 3 stray NACKs. Many of these casual observers inflated the numbers on
> the utxos.org site [4] signalling support for a soft fork activation
> attempt.
>
> Anyone who thinks that maintainers only look at the numbers of (N)ACKs
> is delusional.
> As I explained above, there is a whole lot more nuance to determining
> even just the status of the opinions on a PR, nevermind the code itself.
>
> In this specific example of a soft fork, there is also consideration of
> the opinions outside of the repo itself, such as on this mailing list
> and elsewhere that people discuss soft forks.
>
> On 04/19/2023 11:17 AM, Aymeric Vitte via bitcoin-dev wrote:
>
> > While some simple changes can allow bitcoin to surpass ethereum, as
>
> usual, like "Allow several OP_RETURN in one tx and no limited size"
> https://github.com/bitcoin/bitcoin/issues/27043
>
> > How long it will take remains mysterious
>
>
> No one (maintainers or contributors) is obligated to implement anything.
> A feature request not being implemented is because the people who do
> open PRs are either not interested in implementing the feature, or are
> working on other things that they believe to be higher priority.
> If there is a feature that you want, then you will often need to either
> to it yourself, or pay someone to do it for you.
>
> Additionally, a feature may seem like a good idea to you, but there are
> often interactions with other things that may end up resulting in it
> being rejected or need significant revision, especially for something
> which affects transaction relay.
>
>
>
> Andrew Chow
>
> _______________________________________________
> bitcoin-dev mailing list
> bitcoin-dev at lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev
Michael Folkson [ARCHIVE] /
npub103…2kpam
2023-06-07 23:20:41
in reply to nevent1q…ylr9
Author Public Key
npub103ycruxnchhvja33mcnnkfdkgd0s7vlqlfkvufcdm5lnhpuh6f4q82kpamPublished at
2023-06-07 23:20:41Event JSON
{
"id": "4c3662bb8bfad93752cdf09a79f0203bfb6e4765b78c33788839fd7f9d333e37",
"pubkey": "7c4981f0d3c5eec97631de273b25b6435f0f33e0fa6cce270ddd3f3b8797d26a",
"created_at": 1686180041,
"kind": 1,
"tags": [
[
"e",
"5dceb99394b9e8fe409f15f1d200290ea749436ed7f29c12cb94147296ad7609",
"",
"root"
],
[
"e",
"ad7ff64a91e71318ef5656850be144330aeb5b5926416a6d09bf32efffc7c492",
"",
"reply"
],
[
"p",
"4a273da3c9ab85c096f859e6ca066d2fdfe762406cadc2f4d58aa75468aca8d0"
]
],
"content": "📅 Original date posted:2023-04-20\n🗒️ Summary of this message: Public communication and explanations from reviewers and maintainers can improve the transparency and scalability of the open source project. Lack of communication can lead to confusion and difficulty for new contributors. The departure of long-time maintainers may also contribute to delays in merging pull requests.\n📝 Original message:Thanks for this Andrew.\n\n\u003e What commentary does there need to be?\n\nThere doesn't \"need\" to be explanations about anything. There doesn't \"need\" to be any review comments whatsoever from anybody. But a world where reviewers explain what they've done to satisfy themselves that a pull request is ready to merge and a world where maintainers explain their thought process behind a merge decision is much easier to follow and much more scalable than the current black box where people see pull requests being self merged by a maintainer with no ACKs within a day of it being opened. Most likely these decisions make sense (low risk, unlikely to be reviewed by anybody else, blocking other pull requests etc). But more and more people are funded to work on Core and increasingly they seem to stick to their own mini projects and not review anybody else's work. Of course you can't put the responsibility for this entirely down to maintainers but the black box isn't scalable. Maintainers (presumably) have private discussions and so know how best to spend their review time. Everyone else (especially new contributors) are playing an uninformed and in the dark lottery with how they spend their review time (to the extent that they allocate any).\n\n\u003e There are currently 320 open PRs and 366 open issues.\nI wake up every morning to 150+ email notifications containing\neverything that went on overnight, and throughout the day, I typically\nget hundreds more.\n\nIndeed. All the more reason for more and higher quality public communication. If you struggle and you're in those private discussions with other maintainers on merge decisions and ready for merge discussions how do you think everyone is trying to assess how to spend their time? It is even more bewildering. As far as I know most of the current maintainers haven't worked on other projects or in the private sector for a sustained period of time but the way other projects and businesses function is that those with the most experience and most responsibilities are able to manage and provide guidance to those with less experience and less responsibilities. I'm sure this goes on within Brink if you've been anointed but this is supposed to be an open source project. If everything is done in private conversations and everything other than the code is open source it is pretty much a façade. It is very hard to make meaningful contributions without getting anointed. Those who do get anointed very early in their careers seem especially bad at hoarding information, refusing to do anything in public and not assisting those who haven't been anointed.\n\n\u003e Things can simply fall through the cracks.\n\n\u003e With several long time maintainers stepping away, this may be a factor\nin PRs taking longer to get merged as the remaining maintainers may be\nless familiar with the parts of the codebase that were previously\nmaintained by someone else.\n\n\u003e Requiring maintainers to have to write explanations for every single\nmerge is simply going to increase the burden on them and increase the\nrate of burnout and resignations.\n\n\u003e We've had too many maintainers step down already.\n\nThis all points to a a need for additional maintainers (assuming they are sufficiently competent and qualified). We had a potential maintainer (Vasil) come forward (long term contributor, made significant contributions over a number of years, a qualified reviewer, contributes to a part of the codebase that current maintainers aren't very familiar with, ACKed by maintainers and long term contributors) and it was blocked. How does that make sense? You seem to want it both ways. We can't ask maintainers to meet higher standards because there's a shortage and they're close to burning out. But there's no need to add a new maintainer.\n\nI've responded to the parts I disagree with and see inconsistencies with but generally I thought it was a very thoughtful and informative response so thank you. Of the current maintainers you seem to me to be one of (if not the) most responsive and open to public discussion on the project. I've learnt a tonne from your StackExchange posts and Twitch streams that are all public/open source that you do in addition to your contributions and maintenance of Core. \n\nThanks\nMichael\n\n\n--\nMichael Folkson\nEmail: michaelfolkson at protonmail.com\nKeybase: michaelfolkson\nPGP: 43ED C999 9F85 1D40 EAF4 9835 92D6 0159 214C FEE3\n\n\n------- Original Message -------\nOn Wednesday, April 19th, 2023 at 22:33, Andrew Chow via bitcoin-dev \u003cbitcoin-dev at lists.linuxfoundation.org\u003e wrote:\n\n\n\u003e Responses in-line.\n\u003e Note that the opinions expressed in this email are my own and are not\n\u003e representative of what other maintainers think or believe.\n\u003e \n\u003e On 04/18/2023 08:40 AM, Michael Folkson via bitcoin-dev wrote:\n\u003e \n\u003e \u003e Communication has been a challenge on Bitcoin Core for what I can\n\u003e \n\u003e tell the entire history of the project. Maintainers merge a pull request\n\u003e and provide no commentary on why they’ve merged it.\n\u003e \n\u003e What commentary does there need to be?\n\u003e It's self evident that the maintainer believes the code is ready to be\n\u003e merged, and has observed enough ACKs from contributors that they are\n\u003e comfortable to do so.\n\u003e You're welcome to ask for clarification, but frankly, I don't think\n\u003e having any commentary on merges is going to be helpful or more elaborate\n\u003e in any way.\n\u003e Requiring maintainers to have to write explanations for every single\n\u003e merge is simply going to increase the burden on them and increase the\n\u003e rate of burnout and resignations.\n\u003e We've had too many maintainers step down already.\n\u003e It'll end up being a bunch of boilerplate comments that don't say\n\u003e anything meaningful.\n\u003e \n\u003e There are certainly situations where PRs are merged very quickly or with\n\u003e otherwise little apparent review.\n\u003e But, as I said, if you ask a maintainer why it was merged, the answer\n\u003e will be \"I thought it was ready and had enough review\".\n\u003e There may be other reasons that made the maintainer think it was ready\n\u003e sooner, such as the PR fixes a critical bug or security vulnerability,\n\u003e but these reasons aren't going to be stated publicly.\n\u003e \n\u003e \u003e Maintainers leave a pull request with many ACKs and few (if any)\n\u003e \n\u003e NACKs for months and provide no commentary on why they haven't merged it.\n\u003e \n\u003e There are currently 320 open PRs and 366 open issues.\n\u003e I wake up every morning to 150+ email notifications containing\n\u003e everything that went on overnight, and throughout the day, I typically\n\u003e get hundreds more.\n\u003e It's impossible to keep up with everything that goes on throughout the repo.\n\u003e ACKs come in sporadically, PRs are updated, reviews are posted, etc.\n\u003e Often times PRs are not merged simply because the maintainers were not\n\u003e aware that a PR was ready to be merged.\n\u003e Things can simply fall through the cracks.\n\u003e \n\u003e Of course there are other reasons why something might not be merged, and\n\u003e these generally fall into the camp of \"I don't think it has had enough\n\u003e review\".\n\u003e It's the maintainer's judgement call to make as to whether something has\n\u003e been sufficiently reviewed, and part of the judgement call is to\n\u003e consider the quality and competence of the reviewers.\n\u003e If a PR had 100 ACKs but all from random people who have never\n\u003e contributed to the project in any capacity, then it's not going to be\n\u003e merged because those reviewers would be considered low quality.\n\u003e It's not just about the numbers, but also about whether the reviewers\n\u003e are people the maintainers think are familiar enough with an area and\n\u003e have had a history of thoroughly reviewing PRs.\n\u003e For example, if a reviewer who primarily works on the mempool reviewed a\n\u003e PR in the wallet, I would consider their review and ACK with less weight\n\u003e because they are unlikely to be familiar with the intricacies of the wallet.\n\u003e Obviously that changes over time as they make more reviews.\n\u003e For another example, if I see an ACK from a reviewer who posts reviews\n\u003e that primarily contain nits on code style and other trivialities, I\n\u003e would consider that ACK with less weight.\n\u003e \n\u003e Furthermore, the maintainers are not necessarily the ones who block a merge.\n\u003e Part of evaluating if something is ready to be merged is to read the\n\u003e comments on a PR.\n\u003e Other frequent contributors may have commented or asked questions that\n\u003e haven't been resolved yet.\n\u003e PRs will often not be merged (even if they have ACKs) until a maintainer\n\u003e deems that those comments and questions have been sufficiently resolved,\n\u003e typically with the commenter stating in some way that their concerns\n\u003e were addressed.\n\u003e In these situations, no commentary from maintainers is given nor\n\u003e necessary as it should be self evident (by reading the comments) that\n\u003e something is controversial.\n\u003e These kinds of comments are not explicit NACKs (so someone who is only\n\u003e counting (N)ACKs won't see them), but are blocking nonetheless.\n\u003e \n\u003e Lastly, personally I like to review every PR before I merge it.\n\u003e This often means that a PR that might otherwise be ready to be merged\n\u003e wouldn't be merged by myself as I may not be familiar with that part of\n\u003e the codebase.\n\u003e It may also mean that I would require more or specific additional people\n\u003e to review a PR before I merge it as I would weight my own review less\n\u003e heavily.\n\u003e With several long time maintainers stepping away, this may be a factor\n\u003e in PRs taking longer to get merged as the remaining maintainers may be\n\u003e less familiar with the parts of the codebase that were previously\n\u003e maintained by someone else.\n\u003e \n\u003e \u003e but a casual observer would have only seen Concept ACKs and ACKs with\n\u003e \n\u003e 3 stray NACKs. Many of these casual observers inflated the numbers on\n\u003e the utxos.org site [4] signalling support for a soft fork activation\n\u003e attempt.\n\u003e \n\u003e Anyone who thinks that maintainers only look at the numbers of (N)ACKs\n\u003e is delusional.\n\u003e As I explained above, there is a whole lot more nuance to determining\n\u003e even just the status of the opinions on a PR, nevermind the code itself.\n\u003e \n\u003e In this specific example of a soft fork, there is also consideration of\n\u003e the opinions outside of the repo itself, such as on this mailing list\n\u003e and elsewhere that people discuss soft forks.\n\u003e \n\u003e On 04/19/2023 11:17 AM, Aymeric Vitte via bitcoin-dev wrote:\n\u003e \n\u003e \u003e While some simple changes can allow bitcoin to surpass ethereum, as\n\u003e \n\u003e usual, like \"Allow several OP_RETURN in one tx and no limited size\"\n\u003e https://github.com/bitcoin/bitcoin/issues/27043\n\u003e \n\u003e \u003e How long it will take remains mysterious\n\u003e \n\u003e \n\u003e No one (maintainers or contributors) is obligated to implement anything.\n\u003e A feature request not being implemented is because the people who do\n\u003e open PRs are either not interested in implementing the feature, or are\n\u003e working on other things that they believe to be higher priority.\n\u003e If there is a feature that you want, then you will often need to either\n\u003e to it yourself, or pay someone to do it for you.\n\u003e \n\u003e Additionally, a feature may seem like a good idea to you, but there are\n\u003e often interactions with other things that may end up resulting in it\n\u003e being rejected or need significant revision, especially for something\n\u003e which affects transaction relay.\n\u003e \n\u003e \n\u003e \n\u003e Andrew Chow\n\u003e \n\u003e _______________________________________________\n\u003e bitcoin-dev mailing list\n\u003e bitcoin-dev at lists.linuxfoundation.org\n\u003e https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev",
"sig": "0fd827a9d3b579dd8d96a33e3abf7e36867a8bf1e129e5002999a29ca9b82587b91942b5fab0bf5346ce3c547e5491966605af0ef40c2d19e754e6a15e0c7bae"
}