Are pull requests bad because they originate from open-source development? • Hacker News Board

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

#51

Post by jiggawatts » Tue, Apr 25, 2023, 9:51 AM UTC

winrid wrote: post #34

jiggawatts wrote: post #3

Earlier quoted context omitted.

As far back as the year 2001, way before Git existed, a small startup I worked for used mandatory code review for all commits. You couldn't just upload whatever garbage and call it a day, it had to successfully compile and be reviewed by a randomly selected peer. These days Pull Requests are used to enable the same workflow.

Was the startup successful? :)

Actually no, and IMHO a significant contribution to that failure was that they had bred a culture of cheating where developers would bypass these controls. For example, one of the managers would get his brother to approve his changes, and vice-versa, even if the code was total gibberish.

Generally, the lesson I took away from that place is that you can blow $10 million in seed money and end up with worthless code if you don't pay attention to the basics. They were in the right market, at the right time, with the right product, and 3-5 years ahead of the competition, but you can't sell non-functioning software.

My favourite example (of many) was that they used a source control system that could cache Java ".class" files to enable incremental compilation at all times, even without IDE support. This was literally a check-box that someone un-ticked years before I got there. I re-enabled it for myself as a local setting override and compile times went from 30 minutes to 5 seconds. That inner-loop speed-up was just insane, I could fix bugs in minutes that would take the rest of the team days.

Speaking of code review, the code that I saw getting "rubber-stamped" was so bad it was unbelievable. One guy spent months working on multi-threaded animation code that I replaced with a single line of code. Which actually worked, didn't stutter, and didn't leak threads, or use threading at all for that matter.

The essentials matter! Proper code review, well set up tooling, hygienic build pipelines, etc...

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

#52

Post by ritzaco » Tue, Apr 25, 2023, 10:32 AM UTC

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…

I think these days the emphasis has changed a bit from "required reviews" (more senior dev or peer signs off on code) to "request review" (developer __wants__ the review, because it's a good learning experience and a way to share responsibility for the code (and any problems it might cause down the line).

Anecdotal, but I feel like I see more teams where that "Merge" button is available to most people, but in general doesn't get pressed until the code has at least one review, whereas before it was often only accessible to a select few, or a review was required before it activated.

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

#53

Post by papito » Tue, Apr 25, 2023, 1:40 PM UTC

"then your version control system is not going to fix that for you."

Uh... Pull requests are not a version control workflow - it's a human workflow. It's also a way for the team to actually LOOK at the code that is going in and to be aware of what changes are incoming.

I mean, that's just one point, there are so many ways in which this view is fatally flawed, I am not even going to... ugh.

I know this is not trolling, but this is so scandalous, it comes across that way. This is beyond a "hot take".

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

#54

Post by regularjack » Tue, Apr 25, 2023, 1:41 PM UTC

guntherhermann wrote: post #19

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" is a nonsense statement. If you trust, you do not need to verify, and vice versa.

People make mistakes.

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

#55

Post by peteradio » Tue, Apr 25, 2023, 1:44 PM UTC

mpweiher wrote: post #11

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

Why do you discount the ability of a bad commit to nuke your application? Should firefighters go into buildings without gear because it would show a lack of confidence in their abilities?

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

#56

Post by mrits » Tue, Apr 25, 2023, 1:48 PM UTC

Cthulhu_ wrote: post #27

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.

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…

"Continuous deployment means your mainline is sacred and always in a releasable state."

Well if that is true why not just stop there?

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

#57

Post by dwheeler » Tue, Apr 25, 2023, 1:48 PM UTC

I disagree: it's about humility. We all make mistakes. Having a review process means we are humble enough to admit it.

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

#58

Post by bluefirebrand » Tue, Apr 25, 2023, 1:53 PM UTC

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…

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

You cannot trust any human to make reasonable choices 100% of the time.

If people are making good choices and writing good code then the PR process should be super smooth and seamless, right?

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

#59

Post by bluefirebrand » Tue, Apr 25, 2023, 1:55 PM UTC

arkaeologic wrote: post #10

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…

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.

The idea is "trust early, verify eventually"

As opposed to "trust blindly"

The opportunity to verify immediately is rarely available.

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

#60

Post by dahart » Tue, Apr 25, 2023, 1:56 PM UTC

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…

> PR is about verifying

This is interesting as a counterpoint to trusting, but it’s funny, I’ve never thought about PRs in either of those terms, not within a team or company anyway. The Pull Request can also be about the code writer asking for review, asking the question ‘is this generally the right way to go?’ The funny thing about projecting ‘trust’ and ‘verification’ onto the process is that PRs simply do not prevent all or even most accidents, automated builds and tests are better at that. PRs can help steer things in the right direction, and they’re good as one of many tools for mentoring people about code they’re unfamiliar with. But PRs can also be used as the opposite of verification: if the reviewer is responsible for verification then PRs can be a CYA where the reviewer is implicated in a mistake along with the writer.