This article recommends using merge requests (I'm not a big fan of the term pull request) only in specific cases: https://martinfowler.com/articles/ship-show-ask.html
Are pull requests bad because they originate from open-source development?
91–100 of 101 posts
Re: Are pull requests bad because they originate from open-source development?
#92Earlier quoted context omitted.
> 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…
It seems like your definition of "churn" is "all those petty and annoying things I shouldn't be concerned with or can be easily automated away". By that definition, of course, churn should be avoided when possible.
However, plenty of useful changes also qualify as churn, for instance:
* renaming stuff
* extracting a function to split a block that may have been fine for the author, but that you found hard to read. And yes, sometimes, reviews don't catch all those.
* Add a comment to provide context to an algorithm after you talked with business
All of these things (and others) are pretty valuable, and the cumulative effect of reducing friction against these changes add up to much better codebases, with less places where no one wants to work because of abandonment. This kind of light touches should take a couple minutes while reading your codebase, not half-a-day because you need to fill a PR and wait for someone to be available.
Re: Are pull requests bad because they originate from open-source development?
#93Earlier quoted context omitted.
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…
Sounds to me that the real takeaway is that the essentials you list don't, in fact, matter if you have incompetent developers.
To me, code review is a tool that can make incompetence a temporary condition.
People have to be open to constructive feedback and want to grow, however.
Re: Are pull requests bad because they originate from open-source development?
#94Earlier quoted context omitted.
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…
Sounds to me that the real takeaway is that the essentials you list don't, in fact, matter if you have incompetent developers.
Having a code review policy is like having speed limit signs on the motorway.
Without enforcement, you can make the argument that speed limits don’t save lives because everyone drives as fast as they can anyway.
Same thing for code review. If it’s not policed by management, then it does nothing.
I saw one place that would send alert emails if a commit was approved too fast, assuming that someone must have had it insta-approved by a friend as a favour.
Re: Are pull requests bad because they originate from open-source development?
#95> "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…
I got into a bit of a fight over whether or not "Merge" should be an option on one's own PRs. "Trust" suggests that a developer should be able to review their own code changes, especially for small/trivial things or "expert" level things that no one else feels qualified to review and just leave open for weeks/months instead of even trying to review it (in turn often making it harder to merge without conflicts).
There's a great value in "every change was peer reviewed by someone", of course. There's also a value in "not every change is significant and sometimes you can trust developers to know that".
You can possibly install the fear of accountability if a bug happens in code that the reviewer was the developer until they want that learning opportunity of a code review.
In my experience, most of the "expert-level" developers almost always would love any feedback anyone wanted to offer because their imposter syndrome tells them it is awful and not to sign off on it, but are also more likely to get blocked if their PRs don't move fast enough.
Re: Are pull requests bad because they originate from open-source development?
#96Earlier 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…
>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…
> That's a good description of most teams
Really? "Arch-enemies whose stated goal is to destroy each other and who are pointing world-ending nuclear arsenals at each other" is a good description of most teams?
My condolences.
Re: Are pull requests bad because they originate from open-source development?
#97In 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…
Big PRs with lots of changes are a smell of their own. If you're seeing this cycle, then PRs are just too big and should be broken into smaller tasks.
Of course you're going to miss stuff and have a long review cycle if you have 20+ files with meaningful changes in a PR.
Re: Are pull requests bad because they originate from open-source development?
#98Earlier quoted context omitted.
>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…
> That's a good description of most teams Really? "Arch-enemies whose stated goal is to destroy each other and who are pointing world-ending nuclear arsenals at each other" is a good description of most teams? My condolences.
It's just natural. In any sufficiently complex organization you have internal conflict and competition for resources, often conflicting goals and adversarial relationships that you have on the outside. Internal conflicts can ironically enough be more vicious than external conflict. Any successful company must have processes in place to function despite of that.
A successful tech company in particular can go from 5 to 500 people in just two years. How do you run on trust when 95% of people don't know each other?
Re: Are pull requests bad because they originate from open-source development?
#99Earlier quoted context omitted.
> 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 pre…
I've had groups where I eventually 'approved' a PR, because... deadlines... even when it wasn't meeting defined standards (primarily around tests/docs). Comments like "please add a test" or "please document X" were just ignored (both inside the version control system and in slack). Eventually, I would merge, and occasionally, we'd hit some bug, trace it back to that, and I would catch blame for "passing" it. Not the…
[flagged]
Re: Are pull requests bad because they originate from open-source development?
#100In 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…
Big PRs with lots of changes are a smell of their own. If you're seeing this cycle, then PRs are just too big and should be broken into smaller tasks. Of course you're going to miss stuff and have a long review cycle if you have 20+ files with meaningful changes in a PR.
I'm specifically talking about small (or reasonable) tasks that get blown up in the PR phase because of perfectionist reviewers.
Requesting for example a big architectural change in a pull request is not a good idea IMO. Ideally it should have happened in the design phase, but overspecifying everything is not the solution. Now, in a company, a request for a bigger change can simply be mentioned in the PR, and the tasks can then be created and assigned as follow ups.
This is usually not possible in OSS; the author of the PR might agree to the necessary changes but later not do them for whatever reason, and there might be no other volunteers to "clean up". Therefore it does makes sense to gatekeep and drag out some PRs in OSS.