Date: Thu, 20 Apr 2023 08:45:58 +0000
To: Andrew Chow <lists@achow101.com>,
 Bitcoin Protocol Discussion <bitcoin-dev@lists.linuxfoundation.org>
From: Michael Folkson <michaelfolkson@protonmail.com>
Message-ID: <ksg0BwOXTwfAcy3CG9ghFvxEb_qo6J7vUKVqDmuWTj74u8HIbXqz3pdCJXnOE_exr6KGxA5j5WVjhmgZvilzBj5jO8qTWfjvH_dcX5ijpoM=@protonmail.com>
In-Reply-To: <feef7f88-b46d-7355-1716-122afc6359ee@achow101.com>
References: <uuq_VbxJp50_-m4ufKpEhJOknhZ0pvK8ioDabCkxtDjBYauO3gLKrj2O2tjS6YIFOnJLyaZg6-LENzom1DyQQ3TyMLIIaGz5IRrzrKB8gRs=@protonmail.com>
 <feef7f88-b46d-7355-1716-122afc6359ee@achow101.com>
Feedback-ID: 27732268: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

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 revi=
ewers explain what they've done to satisfy themselves that a pull request i=
s ready to merge and a world where maintainers explain their thought proces=
s behind a merge decision is much easier to follow and much more scalable t=
han 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 t=
hese decisions make sense (low risk, unlikely to be reviewed by anybody els=
e, blocking other pull requests etc). But more and more people are funded t=
o work on Core and increasingly they seem to stick to their own mini projec=
ts and not review anybody else's work. Of course you can't put the responsi=
bility for this entirely down to maintainers but the black box isn't scalab=
le. 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 re=
view 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 communicatio=
n. If you struggle and you're in those private discussions with other maint=
ainers 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 bewil=
dering. 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 guidanc=
e to those with less experience and less responsibilities. I'm sure this go=
es on within Brink if you've been anointed but this is supposed to be an op=
en source project. If everything is done in private conversations and every=
thing other than the code is open source it is pretty much a fa=C3=A7ade. I=
t is very hard to make meaningful contributions without getting anointed. T=
hose 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 s=
ufficiently 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 codeba=
se 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 stan=
dards because there's a shortage and they're close to burning out. But ther=
e's no need to add a new maintainer.

I've responded to the parts I disagree with and see inconsistencies with bu=
t 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 t=
he) most responsive and open to public discussion on the project. I've lear=
nt a tonne from your StackExchange posts and Twitch streams that are all pu=
blic/open source that you do in addition to your contributions and maintena=
nce of Core.=20

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 <bitco=
in-dev@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.
>=20
> On 04/18/2023 08:40 AM, Michael Folkson via bitcoin-dev wrote:
>=20
> > Communication has been a challenge on Bitcoin Core for what I can
>=20
> tell the entire history of the project. Maintainers merge a pull request
> and provide no commentary on why they=E2=80=99ve merged it.
>=20
> 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.
>=20
> 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.
>=20
> > Maintainers leave a pull request with many ACKs and few (if any)
>=20
> NACKs for months and provide no commentary on why they haven't merged it.
>=20
> 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 re=
po.
> 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.
>=20
> 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 wall=
et.
> 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.
>=20
> Furthermore, the maintainers are not necessarily the ones who block a mer=
ge.
> 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.
>=20
> 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.
>=20
> > but a casual observer would have only seen Concept ACKs and ACKs with
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> On 04/19/2023 11:17 AM, Aymeric Vitte via bitcoin-dev wrote:
>=20
> > While some simple changes can allow bitcoin to surpass ethereum, as
>=20
> usual, like "Allow several OP_RETURN in one tx and no limited size"
> https://github.com/bitcoin/bitcoin/issues/27043
>=20
> > How long it will take remains mysterious
>=20
>=20
> 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.
>=20
> 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.
>=20
>=20
>=20
> Andrew Chow
>=20
> _______________________________________________
> bitcoin-dev mailing list
> bitcoin-dev@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/bitcoin-dev