No wonder the blog author is a consultant. When you're not working on "your" project, blaming production issues on processes is always an option.
Why your team doesn't need to use pull requests
31–40 of 76 posts
Re: Why your team doesn't need to use pull requests
#32> Using pull requests for code changes by your own team members is like having your family members go through an airport security checkpoint to enter your home. It’s a costly solution to a different problem. No, it's like having an editor proofread your article before you submit it. It's a second pair of eyes to make sure you haven't missed something or made a change that will subtly break things, or just misundersto…
Exactly, but it's also important to note that different teams have different requirements, and the QA on those will be different for all.
Pull requests with code reviews obviously make since in a large number of environments. But so does committing directly to a single dev branch for others.
> CI isn't some magical fairy that ensures program correctness
So very true, it much better to see it as a check for regressions or a small number of very well defined core areas. Proper user testing by someone who is trying to break it is so much more important.
Far too often small teams try to use tools and procedures that have been designed for large teams and businesses, when in reality they have been designed to solve organisational issues that small teams don't have.
Re: Why your team doesn't need to use pull requests
#33I strongly disagree with this article. I wouldn't trust myself or my my coworkers to produce perfect code every time. We have 2 reviewers mandatory for each PR and using pull requests is a perfectly acceptable way to ensure quality - every line of code has to be vetted by at least 3 developers (it has to pass the CI/CD pipeline checks as well). If you produce high end software with complex workflows and/or calculatio…
> We have 2 reviewers mandatory for each PR > every line of code has to be vetted by at least 3 developers Those are not equivalent. Reviewers typically don‘t "vet every line of code".
2 reviewers seems like overkill unless it’s they’re also testing actual working. Reading code in a PR is not worth so many people-hours.
Re: Why your team doesn't need to use pull requests
#34I get what is being said. There can be a delay in waiting for a review, but in my experience that's more an issue of bad discipline. I do get the point about pair programming making reviews near-redundant, though I'd be inclined to still get review from a third party. I also think pull requests in github, as an actual place you can go and comment on and discuss code, is such a useful tool for doing reviews in a publi…
My biggest issue is really that github PRs are shit, and especially for large PRs they scale very badly (though that’s also an issue of having large PRs):
- it’s extremely hard to correlate changes to comments, especially when rebase / force push are involved
- long discussions are extremely hard to follow as they’re not threaded, especially as “technical” events get mixed with people talking, you can easily have more noise from technical changes (tags, pushes, issues / PRs linking to the PR, review requests) than you have human comments, and GitHub’s forceful folding of the middle of the discussion makes things worse.
- inline comments quickly make diff views unreadable
- there’s no good / built-in handling of individual concerns to resolve, you can “resolve” inline annotations but you have to visit individual resolved threads and pray the author left some indication as to how they resolved the thing otherwise you’ve got no idea
Re: Why your team doesn't need to use pull requests
#35> Using pull requests for code changes by your own team members is like having your family members go through an airport security checkpoint to enter your home. It’s a costly solution to a different problem. No, it's like having an editor proofread your article before you submit it. It's a second pair of eyes to make sure you haven't missed something or made a change that will subtly break things, or just misundersto…
Yes, (competent) code review is constructive, not gladiatorial.
Re: Why your team doesn't need to use pull requests
#36I respectfully disagree. I worked with "trunk"s before, won't do that again, sorry.
I would argue that Pull Requests (or Merge Requests) are the single most efficient practice to ensure the quality of the code in a project with many people, right after the CI with linters, formatters, tests, etc. which you should set up even if you work alone (at least running them locally). Not to mention the knowledge sharing aspect of PR/MR reviews. It is a very versatile practice where a reviewer (or several reviewers!) can review the code itself from a different perspective and in a different setting, review the design (or "architecture", if you want), learn something new themselves, and keep up to date with the changes in the source code, but also in a product in general. Even in my solo projects I always take five before committing the code, in order to review it first with a clear head.
Re: Why your team doesn't need to use pull requests
#37For us this wouldn't work. PR's are a great way of giving feedback and teaching junior devs. It also gives people people an idea of whats going on with the codebase and provides a way to align code style. Pair programming.. sure.. but PP is so resource intensive and often it's better to have people focus by themselves imho.
> Pair programming.. sure.. but PP is so resource intensive and often it's better to have people focus by themselves imho.
Wholeheartedly disagree on the last part. Yes, Pair programming is expensive, but the knowledge transfer is invaluable. I would encourage people to work together on the same problem.
Re: Why your team doesn't need to use pull requests
#38> Using pull requests for code changes by your own team members is like having your family members go through an airport security checkpoint to enter your home. It’s a costly solution to a different problem. No, it's like having an editor proofread your article before you submit it. It's a second pair of eyes to make sure you haven't missed something or made a change that will subtly break things, or just misundersto…
The article actually touches on other methods of review later on. I agree with a lot of points the author makes: there are other methods of increasing velocity and reviewing code that don't involve costly PRs. But they've become doctrine at many companies, even tiny startups that don't need a lot of process breaking velocity. Know the right time and context.
His alternatives to PRs are pair programming and after the fact reviews (when the code is already in the mainline). I don't see any orgs where we would have wanted that, small or big team.
I think the main disconnect is that, decreasing velocity is the point of a PR in a way. We usually want to stop the dev process to have people look back at what's developped as a unit of code, and judge it with some distance, possibility from another team to create even more distance from the code.
That's where some teams put the checklists, or force a recheck of the requirements to validate the code actually matches what we want, etc.
So sure, if someone only cares about velocity PRs are just a burden, but that's far from a consensus IMHO (I'll show my hand by confessing I open PRs for myself, on my own private project, just to review the code in another context than my editor...)
Re: Why your team doesn't need to use pull requests
#39> Using pull requests for code changes by your own team members is like having your family members go through an airport security checkpoint to enter your home. It’s a costly solution to a different problem. No, it's like having an editor proofread your article before you submit it. It's a second pair of eyes to make sure you haven't missed something or made a change that will subtly break things, or just misundersto…
Also as far as I can tell OP’s recommendation is to break the mainline, then tell everyone it’s broken. > Instead, you frequently - at least once a day - put your code into a healthy state that passes tests and integrate it into the mainline IME that’s a perfect recipe for getting a mainline which never passes tests in any organisation of a non-trivial size, as soon as broken code has been pushed “giving a fuck” goes…
Yes. I work on a monorepo with >1000 other devs, in every time zone. If we just broke mainline all the time, you'd get people merging broken code in Asia at the end of their work day and then we would have to revert it in NA. That would be a nightmare!
Also I wish our CI took 10 min :( As of now it can take up to 90 minutes just to compile the codebase (although caching usually mean it's much faster than that.)
Re: Why your team doesn't need to use pull requests
#40Hm, the section on using CI rather than pull requests seems odd. PRs can be part of CI. You can set up your pipeline to build and test the result of a PR; that is, you are testing each change as if you had pushed it to the project's mainline, but there's no potential for you to break the mainline branch in a way that hinders your team. This is very nearly touched on in the "pipeline approvals" section: "...you place…
I usually have two jobs on every PR: one that builds "as presented", and one that builds after an automatic integration with the parent branch. If the merge fails, you still have test feedback from the "as presented" build.
Depending on how long your builds are it doesn't even really take any extra time, if they're done in parallel