Editing PRs as a Reviewer

Recently, Chives submitted a pull request to make some great stylistic changes to the WhyDRS org's intro page. When I saw them, I added a single-line comment asking if they thought it would be a good idea to format the links in a specific way (they were just plaintext).

There was general agreement between not only myself and Chives but also another community member whom Chives had requested to review the PR. So there were two of us suggesting this change, which I had introduced into the scope of the request, even though it was completely outside (a) the existing skillset and (b) the original intent of Chives.

Then I really started getting centralized when I didn’t see the minor changes made within a day or so. Since I knew it was quite a simple task, I just threw them into a GPT session and quickly pasted the output into a direct branch edit. This single change is what Chives ended up merging.

Stealing Individuality

Firstly, upon lookback, the AI actually removed a small line that Chives introduced as a fun personal tidbit. It was a little more conversational in tone and directly connected with the reader. In my push for efficiency and fast implementation, I accidentally removed this key individual input without cause.

Secondly, this PR was ignored for days thereafter by Chives, despite active involvement on a host of other GitHub items. Even if that latter part weren’t true, there is nothing that should force me or him to be actively working on these maintenance items over a weekend in the traditional labor sense. It was only my lack of patience or fear that things wouldn’t look professional to new users that led me to take control of the PR’s narrative and development.

But these items aren’t “wrong” at all, and they certainly shouldn’t be left up to the reviewing member to act as the sole determinant of what is and isn’t an acceptable contribution. Especially since this repo isn’t dealing with mission-critical systems code deployed across thousands of consensus nodes, there really is no harm whatsoever in just letting the whole thing merge with a little bit of positive encouragement. That’s the style I’ve been using with James, and it worked wonders when they recently replied to a light mention in a commit message within minutes.

Oversight Implications

I just need to, and do, have this great trust that people who do the work are the ones best adapted to maintain and ship their efforts, unless proven malicious. And of course, Chives is not malicious, and these bona fide changes came from an exceptional place of shared development. I think that’s what should be conveyed (and documented) across the history of PRs.

Namely, as soon as maintainers start actively editing PRs for anything materially greater than a small typo, the locus of control radically shifts. It becomes a contributor's job to propose changes and implement them into what is now transforming into a free worker system. In this perverse social construct, anyone can suggest whatever they want the "core" team to implement, leaving the contributor to only set out minimal starting frames in a PR.

If we are to follow this to its logical conclusion, then we would have core engineers working tirelessly on others’ code to complete PRs they did not originate because of the desire to “make the merger item right.” This sounds eerily similar to the central control exerted by a manager when they tell someone to do this or that work item. Thus, it is a very dangerous trap to have maintainers constantly editing PRs to get things “just right.”

Productive Alternative

So this gets into the last part, which I’ve long favored. The idea is to just let people have their little wins and succeed in their own contributing journey. The idea originated when I made a PR to the Stellar Flutter SDK.

This was a particularly enjoyable development experience for me, which I can intimately recall, despite happening years ago. The premise was that I wanted to add 18-word mnemonic seed phrase generation to the app toolkit because I think they are a fun security tool that gives you a leg up ahead of everyone once quantum hits.¹ So I spent a day going through the raw keygen scripts² to implement a crossover between the existing 12- and 24-word implementations.

When I created the original PR merging the work, I had a small variable naming typo. Nonetheless, the repo owner enthusiastically merged everything with a gracious note, which felt really good to me. It was later that I recognized the error and just made another much simpler PR with a fix—and that was quickly approved.

The point here is letting people scope out their own problems and craft their own solutions. Had I let Chives spend a few days thinking about some of the conversation on the PR, they might have come up with an ingenious idea that greatly bettered the whole project. Indeed, they might have added an even more descriptive linking interface that highlighted repositories in an even more creative way—something that’s not far out of the realm of possibility given it was their idea to upgrade the particular profiler document here in a meeting earlier this week.

I should have just let this creativity flow through the ongoing changes they proposed, rather than trying to co-opt and direct the flow of development. A much more appropriate course of action would have been to graciously merge the PR from the onset. Then I could just go in with a new one that made the minor link formatting changes.

I’ve done this ongoingly with Taking Stock episode description items and planning, and it offers material accountability and collaborative benefits. Namely, you can see exactly and clearly who proposed and implemented which improvements.³ This lets everyone spread credit where it’s due and gives the contributor a proper sense of identity when their individual and self-identified work item achieves commitment to the collective group effort.