Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

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

blog.ploeh.dk

Post reply on HN
Print view View original

61–70 of 101 posts

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

#61

Post by PretzelPirate »

madsbuch wrote: post #39

It seems like we are reaching the same conclusions as Dave Farley on our development team. We have pivoted to just let everyone merge in their stuff and do "reviews" retroactively when we stumble upon code not up to the standard we set. The reasoning behind this is exactly that we trust individuals on the team to merge. and because reviewing in the traditional sense seems to be a formalisation of distrust: Something…

I would certainly fail my SOC2 audits if I told the auditor that anyone on the team can merge in any unreviewed change and that gets pushed to production, but not to worry be cause we'll catch any issues eventually.

I trust my peers, but we all make mistakes and we do code reviews because we understand that working together can catch issues sooner. We aren't overly picky or mean during code reviews, but we make sure there aren't any glaring issues, and if there are, we talk about them and work together to fix them.

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

#62

Post by SkyBelow »

coldtea wrote: 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…

>trust but verify

If you have to verify, is there really trust?

We need to get to the point that a lack of trust isn't some inherently bad thing. I don't trust myself to never make mistakes. A PR is a way to get others to add a layer of verification to help catch bugs. I don't even trust the PR to catch all bugs, but it is a cost effective for the amount of bugs it does catch. Why is this lack of trust a bad thing? Humans are imperfect, we should build systems that require verification because we cannot trust ourselves to never make a mistake.

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

#63

Post by ilyt »

coldtea wrote: 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 (not the post author, but Dave Farley, the guy who wrote the above excerpt) has never heard of "trust but verify".

"This guy" sounds like someone that never worked on project size bigger than one. It is utterly idiotic take, on every level.

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

#64

Post by dherikb »

the_gipsy wrote: post #7

In OSS, after merging a PR, generally you can't get back at the author and make them do a follow-up change, because they are volunteers. A certain degree of gatekeeping is necessary. In a company however, you absolutely can just assign such a task to the author or someone else. That means it's possible to integrate changes into the main branch faster, which is very beneficial. I've seen teams that try to mimic OSS de…

I have a similar opinion.

Also if the team is mature enough, the PR can be reviewed after the merge, so the necessary changes can be made with the current code already integrated and already running in production. The downside for this is that the team needs to have some discipline to review the already merged PRs and the developers needs to be disciplined to organize your time to make the fixes after the PR merged.

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

#65

Post by kodah »

serial_dev wrote: post #30
whatshisface wrote: post #20

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…

There are few reasonable choices in programming. There are a lot of choices with tradeoffs, programmers certainly aren't omnipotent of tradeoffs. Peer review also isn't just about you and your code. A fair amount of the time it's a tool for democratizing knowledge or teaching in the same way that reading incidents occasionally is.

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

#66

Post by Illniyar »

PRs might have been an open source innovation, but not "trusting" your developers has a very long history that predates OSS and PRs.

The way it was done was to have QA check your code, usually as part of a release.

I think PRs became more popular in non-OSS because of CI/CD and agile methodologies.

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

#67

Post by SkyBelow »

whatshisface wrote: post #20
mpweiher wrote: post #11

Earlier quoted context omitted.

> 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…

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.

Are those relationships really trusting then? Or is it an optimized way that catches fraud before it is too bad while reducing the increased expense of avoiding fraud?

If you have to get all expenses approved before they are charged to the card, then it adds much more overhead and planning which decreases efficiency. The amount of fraud that happens by letting someone charge things and verifying it later ends up being a smaller expense than the amount of preplanning.

Similar with interviews. The amount of time wasted by interviews of someone who doesn't pass a background check is a small cost than the cost of doing interviews of everyone before doing a background check (not just the cost of the background check, but having a longer interview process leading to missing out on candidates and more candidates getting part the way through the process and dropping out because another place gave them an offer).

Imagine a father trying to "trust but verify" the paternity of their child. If there has to be verification, it will be taken as a lack of trust.

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

#68

Post by ilyt »

pyrale wrote: post #44
Cthulhu_ wrote: post #27

Earlier quoted context omitted.

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.

Disagree. Mixing random style fixes in-between actual code just makes reading changelog more annoying. Sure, if you are refactoring some function put format changes with it but that's the most you should do.

If you want to make those:

* make it in separate bunch of commits * make sure team uses tooling that will not allow new style problems to happen, like mandating certain formatter with certain settings before each commit. Most editors have hooks to do it automatically, and if they don't, Git have them too

It also makes git blame less useful althought it now have options to ignore moved code or whitespace changes so it isn't as much of a deal now.

Post reply on HN