Live data from Hacker News

Why your team doesn't need to use pull requests

infrastructure-as-code.com

11–20 of 76 posts

Re: Why your team doesn't need to use pull requests

#11
post #3

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

Agreed. It's part of my job to review my colleagues code, and I expect reviews from them ideally within a couple hours, maximum 24 hours. I find this practice also helps with knowledge sharing and breaking silos. And while I wait for a review, I can start another task, do some manual QA, grab a coffee, write some docs for this feature, etc. I rarely find myself stuck waiting for reviews.

Re: Why your team doesn't need to use pull requests

#12
For 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.

Re: Why your team doesn't need to use pull requests

#13
post #7

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

Does a change being accepted into master equal it becoming live in production at your company? That seems even more ludicrous.

Re: Why your team doesn't need to use pull requests

#14
> An outsider may not understand the vision and direction of your project. They may not have the same habits and norms for testing, code quality, and style. However, your own team members should share these norms.

Yeah, no. The challenge of running a company is that even within your company, everyone becomes misaligned the moment you’ve aligned. Have you hired anyone new? They don’t understand the company’s vision, nor have internalized your engineering standards. And even once they’ve worked with you for years, they still won’t have aligned on the things that they never ran into, AND they’ll misalign on all the things they've developed opinions on.

> Doing this means that a human only spends time reviewing code that has already been proven technically correct.

Yes, CI is useful for ensuring technical correctness. But technical correctness is one very small aspect of reviewing, and usually not where the big issues come in day to day.

Getting eyes on code is not about preempting things that would fail CI anyway. You probably should be running your full CI suite on PRs, too. To me it’s about catching misalignments on what the product should do, broader misalignments on how it should be engineered, catching misalignments that someone who’s too deep in the weeds would miss, talking through issues realized after the requirements had already been set and discussed, and more. A CI pipeline can’t fix that, pair programming will only address the things that an engineering peer who is as equally in the weeds as the contributor is, and integrating the newest things to production without human review is nice if your product is sufficiently low stakes that subjecting users to the effects of these people problems is okay; but that’s not for me and my team.

Pushing faster is not a pure good without trade offs.

Re: Why your team doesn't need to use pull requests

#15
A pull-request based approach is more sane than changing CI/CD to CI/C-MergeConflicts

"pull requests sacrifice performance, including both delivery time and quality" Delivery time does not matter if your feature is not fully implemented; "Your team members should share your norms for quality" (paraphrased)

The article mentions having "regular, scheduled reviews" which sound like a chore and break flow more than having to review a PR every now and then. Having these on a weekly basis (as the article suggests) while having everyone push daily means you have 5 days of incomplete, unchecked code in your codebase, per engineer, every week.

CI and PRs are not mutually exclusive, you can have your github PR in "draft-mode" and it should still run the same workflows. Have your tests be automated and when the feature is done and passing; you request a review.

Re: Why your team doesn't need to use pull requests

#16
I think it's good to challenge received best practices like this - even if in this case the alternative seems to hinge on something that most teams struggle with even more -- writing sufficient good tests.

I favour a less automated solution: code previews.

The biggest reason I've seen for code reviews taking a long time, is that they're surprising. If an engineer has implemented a full solution without communicating their intention ahead of time, reviewing it is really hard because the reviewer is often starting from zero.

1. Communicate your plan ahead of time

2. Check in with the team whenever meaningful progress has been made, and during/after key decision points (you can use draft PRs for this).

3. When it's ready to land, hopefully enough incremental reviews have been done that the final review is simple box ticking (and yes, perhaps it can be fully automated with CI).

Essentially, I think getting humans to look at your code is important, but it doesn't have to be done in a way that feels like a roadblock.

Re: Why your team doesn't need to use pull requests

#17

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

Re: Why your team doesn't need to use pull requests

#18
It's written to trigger me right?

I have introduced prs on low quality teams and guess what happened?

Higher quality, better learning, faster development because we do not constantly fix bugs in production.

Of course automate as much as possible, make the pr cycle short and explain people that creating the pr is not at 99% and you only wait for the button press but more like 70% and expect learning and small rework after.

Re: Why your team doesn't need to use pull requests

#19
post #4

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

I don't disagree with everything in there, The trouble with pull requests for instance is very real, but yeah, without mentioning the trouble with continuous integration nor the benefits of pull requests, it is just a pretty weak and one-sided article. Especially developers should now better than state 'B is bad because xxx and A is good because yyy so pick yyy'.
Post reply on HN