Date: Wed, 19 Apr 2023 21:33:33 +0000
To: bitcoin-dev@lists.linuxfoundation.org
From: Andrew Chow <lists@achow101.com>
Message-ID: <feef7f88-b46d-7355-1716-122afc6359ee@achow101.com>
In-Reply-To: <uuq_VbxJp50_-m4ufKpEhJOknhZ0pvK8ioDabCkxtDjBYauO3gLKrj2O2tjS6YIFOnJLyaZg6-LENzom1DyQQ3TyMLIIaGz5IRrzrKB8gRs=@protonmail.com>
References: <uuq_VbxJp50_-m4ufKpEhJOknhZ0pvK8ioDabCkxtDjBYauO3gLKrj2O2tjS6YIFOnJLyaZg6-LENzom1DyQQ3TyMLIIaGz5IRrzrKB8gRs=@protonmail.com>
Feedback-ID: 53660394:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [bitcoin-dev] Bitcoin Core maintainers and communication on
	merge decisions
Precedence: list

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=E2=80=99ve 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