DEV Community

Discussion on: Pull Requests Are Slowing You Down

Collapse
 
joelbonetr profile image
JoelBonetR 🥇 • Edited

It depends in the criticity of the project.
It's not the same pushing untested/unvalidated code into production for a silly landing page of your friend's barber shop than doing the same on a landing page of Adidas 10 minutes before a 5 million marketing campaing begins.

Not the same doing that in a community than doing it on a banking webApp.

You can't talk about "costs" without adding context. And this generalization or lack of context is applicable to any of the OP posts:

  • Pull Requests Are Slowing You Down
  • The Problem with Feature Branches
  • Why Mocks Are Considered Harmful

There's no real issue with any of this things IRL and those posts are either from a PoV of working on very little projects or just to gain viewers.

Of course each project has it's needs and is perfectly valid to have a side project in which you push directly into Master without automated testing, while having a more serious one that has tests but pushing code directly into Master, while having tests + PR at work on a more serious project.
Just make sure you choose the correct workaround for the project and be willing to change it whenever is necessary.

Collapse
 
bentorvo profile image
Ben Brazier

These are all problems I have dealt with in production environments of banks, media companies, and more. If you disagree that's fine but please provide reasons other than not providing enough context and that you don't think it's a good idea.

Thread Thread
 
joelbonetr profile image
JoelBonetR 🥇 • Edited

Sorry I thought it was self explanatory.
In security it's said that a system is as secure as it's weakest node.
Avoiding review steps in critical systems will speed up the process for sure but is not worthy in comparison with the estimated costs of any human error in the process.

Now imagine something big happens by a little mistake, and don't try to convince me about that, try to convince a judge about not using security checks because "they slow you down".

If I think of it you know what? Unit tests are slowing me down as well, already tested my feature/change and it works in my machinem, we can ditch them apart. Integration tests? Meh, tehre's QA environment, why bother? Meetings? That's much time, I rather code on my own without sync-ing with the rest of the teams.

Enough sarcasm for today. The pair programming thingy reminds me to those group exercises at college where some people won't do anything so IRL with that approach they'll be unnecessary resources.

The middle point on that is to apply branch permissions properly. If you have a team with 2 seniors in which you can trust and 2 juniors in which you can't, you can give those seniors permissions to merge/push directly into develop while keeping the PR process for those juniors, which is something more accurate with the reality and real project demands.

You rely a ton on your tests according to your posts. That's fine but remember that tests are done by people and also can include mistakes or not cover all the usecases.

We should remember that there are business around our code and we get paid to solve business needs. Now sum the fact that both in IT and business the thing is to be as much efficient and reliable as possible. A software engineer is paid 2 to 4 or 5 salaries worth of a low qualified job, that's because we can automate those jobs and the acceptance criteria for the product is usually being faster and more reliable than the humans it replaces.
Software is an investment for already existing business or a product itself for digital companies.

If you were right no single company would like to apply this PR process. But again in this platform (as almost every week) I need to remember that there's no [tech, methodology... whatever] to rule them all. You need to analyse your specific use case and choose wisely (If you're in a position to choose) because it's then your responsibility as well.

Cheers

Thread Thread
 
bentorvo profile image
Ben Brazier

I'm not saying don't do review, I'm saying don't use PRs for review as there are better alternatives.

The fact that you will suggest seniors should push to a develop branch indicates that you are avoiding PRs because they are slow. This article wasn't talking about how we should train juniors.

Yes, relying on tests is normal in modern software development.

You rely a ton on pull request reviews according to your responses. That's fine but remember that pull request reviews are done by people and also can include mistakes or not cover all the usecases.

Thread Thread
 
joelbonetr profile image
JoelBonetR 🥇

Perfect, so we can agree on having PRs + Having tests are two checks and avoiding one of them will deal, allegedly, to a less reliable result consequently, let me explain why pair programming is not better than PRs:

First of all, pair programming means 2 resources on a single task during the entire development. Is much faster to check some development than developing it.

Now having a PR means you got the entire change history for that feature branch on a single point, being able to download it and execute it in your machine, checking the code in your IDE while keeping the possibility of calling the teammate for him to explain things if needed. You can double-check the tests, add some if you feel it necessary, execute them in that specific code version and so on.

What can be better than that?

If I'd to choose between both I'd rather pick PRs, but having PRs are not an exclusion for being able to pair programming or pair/peer review the code.

And no, relying on tests is not normal. Neither it is relying on PRs. We usually use both while adding several layers for quality assurance and security in the middle.

It's not me (or any dev) who's slowed down by PRs, it's the product, and past certain point, you need to choose between speed and reliability.

Collapse
 
jesterxl profile image
Jesse Warden

There is nuance, sure, but there is also a ton of compelling scientific evidence; this isn't "little projects". Check out any of Dave Farley's videos/posts on CICD, specifically the pair programming and feature branch ones. The state of devops and other scientific reports have studied these things and have super compelling numbers and quality increases from avoiding feature branches and mocks. If you want to say they're not compelling, you need to provide evidence to the contrary, not confidence.

Tests such as unit, integration, end to end: important and non-negotiable. PR's, for a team of devs working on the same project who trust each other, not needed. We're not saying remove tests or any quality gates in CICD, just replacing PR's with pair programming and small, frequent check ins.

Thread Thread
 
joelbonetr profile image
JoelBonetR 🥇 • Edited

Can you please link those studies with super compelling numbers?

By the way surveys are not "scientific reports" and "the state of devops" is a survey, which I read from 3 different sources (GitLab, CircleCI and Puppet) and none of them mention any of this.

As personal opinion I'd leave asap from a company that expects me to be on call the entire day for pair programming. Am I in a call center?

Avoiding PRs? Yes it can be OK if your team is composed entirely of seniors, I'm on that situation right now and PRs are a secondary thing. So it's a "yes... BUT".
Replacing PRs with pair programming is not trusting, is a waste of time. Sometimes you got good professionals to work with and sometimes you face people that need mentoring and supervision and I prefer PRs and pair programming when needed than being the entire day pair programming for no reason. They need to learn to be autonomous as well.