Why your team doesn't need to use pull requests
infrastructure-as-code.com
Why your team doesn't need to use pull requests
1–10 of 76 posts
Re: Why your team doesn't need to use pull requests
#2[dead]
Re: Why your team doesn't need to use pull requests
#3I 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 public environment where everyone on a team can go back and refer to the discussions that I'd be inclined to accept some amount of delay as an acceptable cost.
Re: Why your team doesn't need to use pull requests
#4I 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 calculations, pull requests are essential.
Re: Why your team doesn't need to use pull requests
#5In our setup, the committer can merge their pull request after CI passed and at least one review has approved the code. Seems like a better compromise than relying solely on CI. And pairing is nice, but there's not always someone available to pair with. Heck, I always review my own pull requests completely before requesting a review, because I often miss something. And the easiest way to do this, that I know of, is with pull requests.
Re: Why your team doesn't need to use pull requests
#6> 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 misunderstood the requirements and implemented something not quite right, or didn't implement your tests properly and thus are not actually testing what you think you are.
CI isn't some magical fairy that ensures program correctness; it's only as good as the tests it runs. And even the best tests do not guarantee correctness.
Re: Why your team doesn't need to use pull requests
#7Is the author suggesting every commit goes into the main branch, and into production? That seems ludicrous.
The trick with merge requests is to keep them as small as possible while being something that can potentially be deployed into production, ie some kind of fix or feature, not just dead code.
Re: Why your team doesn't need to use pull requests
#8Some good points! There are, however, also changes that you do not want to propagate to your teammates or production unless you are 100% sure that they are the final solution to the problem you are trying to solve. Sometimes, the complexity of a problem is so high that you have to experiment with the code.
When those changes include non-reversible database migrations, for example, it is better not to integrate that code into "mainline" before you've settled on a specific implementation.
Re: Why your team doesn't need to use pull requests
#9Strange advice.
Git was designed around the idea of a clean history. Now, one might claim that "designed around the idea" does not necessarily mean that it doesn't work if you don't keep a clean history. However, there's a large amount of tooling within git which outright breaks if you don't stick to a clean history. As such, I am just going to go out and state that if you don't want to keep a clean history, you're at best not getting as much out of git as you reasonably could, and at worse, getting less out of git than if you were to use an alternative which was designed around the idea of not keeping a clean history (e.g. fossil).
In summary, even though I often see software projects already doing a bad job of maintaining a clean history already (which is partly because of pull requests being the wrong tool for small change sets (see kernel development for situations where pull requests make sense)), this approach is even worse.
Want continuous integration to work best? Split up the work into smaller parts which can be contributed as smaller individual units (whether you use PRs or some alternative system). Continuously pushing incomplete changes (even if you round them off so tests pass and nothing immediately breaks) is just not compatible with git.
Re: Why your team doesn't need to use pull requests
#10Where I work we have two code bases. The older of the two in TFS and the newer in git. In TFS we use trunk-based development and in git we do pull requests.
I must say that I very much prefer having a formal, tool-protected, review process, doing integrations and testing on a separate branch from the main branch to allowing developers like we do in git to the added ease of being available to commit directly to trunk.
Even with CI to run tests upon committed code you never want to leave your mainline branch in a broken state. No matter how fast the feedback is, because you don't really know how fast your developers will be to act on that feedback!