Hacker News Reader

Comment by c7DJTLrn | Hacker News Reader

I think many developers treat PRs as an opportunity to gatekeep. I'm not a very politically correct or emotional person but I think gatekeeping/nitpicking is extremely demotivating and it should be avoided wherever possible.

When I review PRs, I'm looking at the big picture. I'm looking to see if it introduces security issues or diverges from standard practice. I don't care if there's a slightly better way which reduces the number of branches - if I see it as worth changing I'll do it myself after the merge. I'm not looking for all the reasons why it can't be merged, I'm looking for the minimum changes needed to make it comply with the codebase's existing standards.

This post also highlights one of the reasons I fell out of love with Rust. There's too many ways to do things.

View on HN

I think a lot of people treat the merge of a PR as their god-given right. Because they spent time on it. But if they spent the time without discussing with the maintainer first, why would they be obliged to merge it ?

It is like spending a ton of money on a big, bulky, but useless to a recipient present, giving it to someone and then complaining that they don’t take it - you had spent all this money!

The way out of this is to be more proactive and discuss first and then get to coding. Oh, and be prepared you would need to code an approach different from what you wanted.

And there is absolutely no shame in forking the repo, doing the changes you could not get merged, and keeping it around for your own/your org use.

It is all about tradeoffs.

>I think a lot of people treat the merge of a PR as their god-given right

I've never seen this at work or in open source, and I've done quite a bit of open source. Contributors will get frustrated if their PR is simply ignored and I think for good reason. But I've never seen anyone say or even imply that their PR must be merged.

I have seen this “non-zero” times - not common, but it exists.

Some folks are insistent that their PR and their specific problem be addressed by your project, and the effort they put into writing code before discussing it just increases their attachment.

The analogy that popped up in my head reading these threads is someone buying expensive and bulky yet useless for the recipient gift and then being upset when it is rejected.

“But i paid a lot for it!”

I am judging it by the comments.

I have been on both sides of this situation over the years, but it was never too extreme.

On the maintainer side: I have a small project that has picked up some pace recently, though is still rather small, and there has been only two situations where i said “this feature is not going in no matter what”. Because it will increase my burden as a maintainer and main author.

On the contributor side: If the maintainer in a timely fashion suggests some changes that seem reasonable, I am happy to spend the time doing them. Often they will give an “outside” view that will make a patch better.

Sometimes I saw them being quite selective about what they want to merge, whereas for me “just that bit” solves a real need and I see the request as too much work - then I just leave my fork as is and use that for what I needed. I see their rationale, and I am thankful for what they have done. And I leave my modifications in the fork in case someone needs them or wants to pick them up and improve.

The freedom with open source always should go both ways.

I've run into this myself: a contributor submitted a pull request that didn't follow a bunch of guidelines, and the changes they introduced were very different from the rest of the code in the same file. I asked them to keep things consistent with the existing code, and they basically argued "This fixes the problem, so you must merge it".

Literally this article?

> if I see it as worth changing I'll do it myself after the merge.

100% agree! Someone unfamiliar with the code base takes the time to make a change that improves something, for free and without asking for anything in return... I always try to take it as it is, only asking for changes that are really necessary to avoid bugs or serious problems, then I normally go and do a post-merge cleanup to try to keep things organized in the way I want (which is hard for outsiders to do themselves).

> There's too many ways to do things.

Exactly! That's why Rust PR reviews are always dangerously close to descending into nit-pick shit shows - there's ALWAYS a 0.1% nicer way to write something, you can always change it to make it 2% more efficient, or to avoid allocating as much memory, or avoid using explicit lifetimes, or avoid dynamic dispatch.... the list goes on.

> I think many developers treat PRs as an opportunity to gatekeep

Well, it is your project or your team's project. It is entirely legitimate gatekeeping.

Questioning that is the bad side of the "social coding" touted by Github. IMO "social-anything" is like the Force in SW, it is easy to fall for its dark side if you don't keep your emotions in check.

> I think many developers treat PRs as an opportunity to gatekeep.

An open-source maintainer should be gate-keeping the project.

Open source is great in that it enables many people to suggest improvements, but the PRs should be viewed as a proposal for collaboration.

Not all PRs are created equal. Far from it. They span a very wide range of quality. Letting every PR into the project will quickly accumulate a lot of technical debt and place even more burden on the maintainers to do further work to undo the backwards steps of the bad PRs.

When keeping quality high (or even just average), gatekeeping isn't inherently a bad thing.

The whole point of PRs is to gatekeep. They are there to enforce that the change is consistent with the owner's standards.

“Gatekeeping” is a requirement for good software. One man’s nitpick is another man’s deeply held value, especially if one is a newb and one is a maintainer and/or has a legitimate long term vision for the project.

Not saying this is the case here but there are two ways to look at it.

What about adding it to the PR yourself? Why should it be the same person creating/fixing the PR?

I'm assuming you mean't "why shouldn't". Because a lot of critique in programming is a matter of personal opinion and I see it as impolite to block somebody else's hard work because it doesn't look the way the reviewer wants. Of course, it's a different matter if the code is insecure or something but I really don't care about the extremely fine implementation details.

Reply on news.ycombinator.com