Live data from Hacker News

Are pull requests bad because they originate from open-source development?

blog.ploeh.dk

41–50 of 101 posts

Re: Are pull requests bad because they originate from open-source development?

#41
I don't really have any opinion about PRs versus trunk-based development, but I do have a small insight that I think is useful when you're thinking about which one to choose. The main advantage of trunk-based dev is speed. You don't have to wait for a review of work. You're trusting the team to merge things that won't break stuff. That's awesome, but if that's the case then you should also be able to trust them to review PRs quickly enough that they don't block work. Equally the converse is true - if you can trust your team is motivated enough to review PRs quickly they you can probably trust them to do work together well enough not to really need PRs.

If you can trust the team with one and not the other then something is wrong. There shouldn't be any "this way is better than this other way" argument. Your team should be capable of doing both. If it isn't then neither will work very well. You're going to have problems, conflicts, bugs, etc no matter what you do.

Believing that the team should stop PRs because they're too slow, or they should stop trunk-based dev because pairing takes too much effort, are both red flags. They're both saying that the team has a problem to address.

Re: Are pull requests bad because they originate from open-source development?

#42

Earlier quoted context omitted.

Here are some examples of trust-but-verify relationships at work: - You are allowed to expense things while traveling, but someone will look at what you put on the company card afterwards. - They don't wait on your background check before moving ahead with interviews. - Someone really should look at your code, at some point, before it is released.

This is how I see the pull request example with credit cards and traveling. You are on your way to a conference. You are at the airport and want to buy a coffee and a donut. You make a request... You've been waiting for almost an hour now. Nobody takes a look at and you still need multiple approvals, so you ping Barbara in charge of approvals to get the ball rolling, she says she will take a look after a meeting she…

I don't assume my colleagues will always make "reasonable" choices and furthermore correctly express those choices into code any more than I will myself! In fact perhaps most of the issues I find in PRs are after reviewing my own ones (often it's silly things like accidentally including files/commits I shouldn't've have). Plus knowing someone else must've approved my PR stops me feeling quite so silly if it does end up breaking something :) The breakfast order analogy to me falls down because not being able to merge a PR into the mainline branch immediately is exceedingly rarely something that holds me up - there's always plenty of other stuff to do. And the time I spend reviewing PRs is valuable in its own right, even if I find nothing wrong (understanding the codebase better etc.). Sure the odd PR or two for a truly trivial change slows up the works a little but they're pretty rare and a it's a small price to pay. Having said all that, I'm genuinely interested in what studies might have been done to genuinely guage the effectiveness of the PR/review process. I've certainly worked on some (open source) projects where it clearly was a major drag on productivity, as very few people had approval rights (wherever I've worked professionally all devs have the same rights - single approval from any other dev is always sufficient).

Re: Are pull requests bad because they originate from open-source development?

#43
post #2

> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…

Just a small note: "trust but verify" is not a reasonable statement, and it was also unreasonable in its original context, which I encourage anyone to research. If you can verify, trust is not required.

How do you verify, that the requirements where correctly understood, if not by having another human look at it?

Re: Are pull requests bad because they originate from open-source development?

#44

Earlier quoted context omitted.

Here are some examples of trust-but-verify relationships at work: - You are allowed to expense things while traveling, but someone will look at what you put on the company card afterwards. - They don't wait on your background check before moving ahead with interviews. - Someone really should look at your code, at some point, before it is released.

And to extend the last point: before it is merged, to keep the mainline in a releasable state: 1. Continuous deployment means your mainline is sacred and always in a releasable state. 2. Unit tests and automated verifications only tell part of the story 3. Churn should be avoided (e.g. fixing code style issues); if fixes are made before it enters the mainline, it is avoided. 4. Code reviews should be asynchronous, be…

> Churn should be avoided

One of the big Taylorist mistakes of our industry IMO. It is OK to have churn, and it's certainly better to have a little too much than too few.

Re: Are pull requests bad because they originate from open-source development?

#45
post #28
post #2

> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…

Also, when you're working solo, PR's are just a great way to look at a bunch of changes before you smoosh everything together because even with a full history it's sometimes tricky unpicking everything.

I believe in this context, PRs refers to a process in a team about steps to check code that goes into trunk, not to the tools that were developed to do the task.

Re: Are pull requests bad because they originate from open-source development?

#46
It's fine to allow some senior engineers to merge a pull request without going through CI and/or code review, but the discipline the PR process introduces is highly desirable, whether working on open-source or not. It's not a question of trust, it's a question of avoiding unchallenged assumptions and blind spots.

Re: Are pull requests bad because they originate from open-source development?

#47

Earlier quoted context omitted.

Here are some examples of trust-but-verify relationships at work: - You are allowed to expense things while traveling, but someone will look at what you put on the company card afterwards. - They don't wait on your background check before moving ahead with interviews. - Someone really should look at your code, at some point, before it is released.

This is how I see the pull request example with credit cards and traveling. You are on your way to a conference. You are at the airport and want to buy a coffee and a donut. You make a request... You've been waiting for almost an hour now. Nobody takes a look at and you still need multiple approvals, so you ping Barbara in charge of approvals to get the ball rolling, she says she will take a look after a meeting she…

If your entire process is broken and toxic, pull requests aren't going to work for you, but that's not a problem with pull requests, that's a problem with your entire process being toxic and broken.

> You make a request... You've been waiting for almost an hour now.

1. Don't design your process so that waiting an hour to merge a pull request is going to be a problem as bad as waiting an hour to eat. Why are you waiting? Move on to the next thing. If it depends on the pending pull request, branch the pending pull request, and work on the new branch--if you have to integrate feedback you can rebase it into the new branch. Occasionally this will cause problems, but they're usually rare.

> Nobody takes a look at and you still need multiple approvals,

2. Choose a number of approvals that is appropriate for your team. If you're writing software where the consequences of a mistake are very high that might be a high number, but in most cases in my career, 1 has been a perfectly reasonable number of approvals. In some cases, 0 is the appropriate number of approvals.

> so you ping Barbara in charge of approvals to get the ball rolling, she says she will take a look after a meeting she is currently attending because she wants to be careful with what she approves.

3. Don't make one team member in charge of approvals--that's almost guaranteed to make that person a bottleneck. Anyone on the team with the knowledge to evaluate the code effectively should be able to approve it. I've been on one team where one guy was in charge of approvals and wasn't a bottleneck, which was because he did nothing but approvals. That worked great for everyone except him--his life was miserable until we scrapped that idea.

> Oh, Tom, her colleague asks you why don't you eat salad and orange juice, it's cheaper and faster. You have a quick call with Tom to explain why you want a coffee and donut.

4. If this sort of exchange is about personal preferences like what you eat or some stylistic nitpicking, then you have to have a team discussion about making sure that we prioritize code review feedback that's actually important and not bikeshedding. But in a lot of cases, Tom is asking because he legitimately doesn't know, and explaining your reasoning to Tom is part of your responsibility to help your teammate keep their understanding of the codebase up-to-date. This isn't wasted time, it's one of the benefits of code review.

> You arrive, by the time you land Barbara approved your breakfast, but now you are in a new country and circumstances changed, so you update your request. The earlier approvals are now invalid.

5. If different pull requests depend on each other or are touching the same areas of the code, ideally do them serially rather than working on them at the same time. If they really need to be worked on at the same time, then you need to communicate earlier in the process to be sure you aren't stepping on each other's toes, and it would make sense for the two people working on the same areas of code to review each other's pull requests. Not doing pull requests wouldn't fix this--this is just a challenge of multiple people changing the same codebase.

> Pull requests assume you cannot trust your colleagues to make reasonable choices

Sometimes reasonable choices are costly mistakes.

Even the best coders make mistakes, and unlike a mistake on what food to eat for breakfast, code mistakes can be very costly.

> and you also don't trust your automated tests to catch anything insane

Automated tests don't catch when your architecture is bad, or when there was a mistake in the specification, or when you've introduced a security vulnerability. Sometimes the mistake caught in code review is in the automated test.

> you have to gatekeep and slow down your team to make sure (?) that the code they commit is good.

Good code can always be better. I, for one, want people looking at my code to make it better. Obviously there are some tradeoffs here, and code review shouldn't be a massive bottleneck in your process. Done well, code reviews speed up your team: the earlier you catch mistakes the quicker they are to fix.

Re: Are pull requests bad because they originate from open-source development?

#48
post #16

Earlier quoted context omitted.

> I cannot imagine how this scenario could work with git though. I cannot image how this scenario could not work with Git†. Maybe I’m missing something important about Subversion, but it’s also just plain old version control. Nothing fancy like Darcs. †: Granted, there is a (theoretical) point where the frequency of pushes becomes so high you can no longer reasonably work on a single branch.

The big difference between SVN and Git is that SVN is centralized (a single linear history is enforced at all times for everyone) and Git is distributed (each user has its own local history). Both models have advantages and disadvantages, but for many people working on the same branch the centralized model is arguably better. (in the end, both Github and Gitlab workflows mainly try to put more 'centralization' back i…

Most corporate git workflows using git are also centralized : the team has one upstream, and people's local repos function as mere clients to check out the upstream. Few teams pull from each other's repos.

If you don't branch and directly push what you commit, there's little difference between SVN and Git.

Re: Are pull requests bad because they originate from open-source development?

#49
Isn't there a bit of an elephant at the other end of the room?

PR are not only about placing a bouncer at the club door to, figuratively speaking, check entering members for excessive drunkenness, they are about placing a bouncer at the door to give access to a wider group.

This is quite obvious in open source, where the PR model has been a revolution in terms of lowering barriers of entry. You can join the party without asking for the keys. But similar barriers exist in companies. When they don't exits on the technical level (they usually do, sometimes to the point where collaborating teams wouldn't even know what version control the other side is using), they exist on the social level and a model like PR/MR can help nudge both towards the "internal open source" mindset.

Re: Are pull requests bad because they originate from open-source development?

#50
post #2

> "Pull requests were invented to gatekeep access to open-source projects. In open source, it's very common that not everyone is given free access to changing the code, so contributors will issue a pull request so that a trusted person can then approve the change. "I think this is really bad way to organise a development team. "If you can't trust your team mates to make changes carefully, then your version control sy…

> This guy ... has never heard of "trust but verify" I think he has, and actually knows where it comes from and what it means. "Trust but verify" is a Russian proverb that was used to describe the hoped-for and treaty-mediated relationship between the Soviet Union and the United States during the Reagan-Gorbachev era. So arch-enemies of a Cold War that are pointing world-ending nuclear arsenals at each other and slow…

>If that's a good description of the relationships in your team

That's a good description of most teams, open or closed, private or public. Hierarchies are universal, for good reason. I'm not entirely sure what hippie commune David Farley programs in where Intern #15 can just merrily hit the big red nuke button and send everything into production

In open source projects pull requests are great because you don't trust people you don't know, in private companies you don't trust people because you know them. Hell I don't trust myself enough and I'm glad processes like this exist

Post reply on HN