Why Pull Requests Are A BAD IDEA

i start quite a high proportion of my videos here with the statement along the lines of software is complicated and i mean it i think that one of the mistakes that we often make in software is to believe that things are easy coding is sometimes easy software really isn't what i mean by that is that i think that we need to proceed with more care then we are nearly always in the situation of running with scissors so let's just be a little bit more cautious while we're doing that this means that i'm a big fan of code reviews amongst other things but i also see that the commonest way in which there are practiced pull requests as something of a problem for software development so let's talk about that [Music] hi i'm dave farley of continuous delivery welcome to my channel and if you haven't been here before please do hit subscribe and if you enjoy the content today hit like as well i'd like to begin by thanking our sponsors equal experts octopus and spec flow they all support our channel and please support them in return by checking out their links in the description below if you'd like to learn more about continuous integration and how it fits into a continuous delivery process check out my training course anatomy of a deployment pipeline which explains both in some detail and it gets great reviews i talk a lot about the value of continuous integration on this channel continuous integration is the idea that we can do a better job if we maintain a clear picture of how our changes work alongside everybody else's as they progress i have several videos on this topic mostly pointing out the risks of hiding changes away from for too long the evidence is fairly clearly on my side in the most definitive study of on software development practice that i'm aware of the state of devops report from the dora group at google their findings are plain if you don't check that your changes work alongside everybody else's at least once per day then statistically you produce lower quality software more slowly that's not me saying that that's based on surveys of tens of thousands of contributors you may argue that the survey may be wrong you may argue that the measures of throughput on st and stability that form the basis of this study are not important for some reason but if we are to approach this from a rational perspective then the way that you show that those things are wrong is there evidence do you have data that counters the findings of the surveys or can you show that the efficiency with which we create systems and the quality of the systems that we produce doesn't matter since these are the measures that the survey is based on this has to be more than only personal experience or the experience of your team that's how statistics work statistics don't say this never works they say this almost never works so saying that your aunt smoked 40 cigarettes a day for her whole life and lived till her 90s doesn't tell us that smoking is good for you it says that your aunt was rather lucky similarly saying that feature branching works for your team doesn't tell us that feature branching works best your team is just rather lucky to find repeatable patterns we need to analyze lots of teams and see what the statistics tell us here's a quote from the devops handbook the data from the state of devops report is clear trunk based development predicts higher throughput and better stability and even higher job satisfaction and lower rates of burnout if you want to disagree with this in a rational rather than an emotional way the way forward is clear come up with or point to some evidence that demonstrates where this is wrong you're probably going to need a survey of over 50 000 response respondents though or a controlled experiment that shows how these statements are wrong i talk about how to approach these sorts of problems rationally in this video so the data says that continuous integration and by that i mean specifically trunk based development works best the commonest pushback that i get at this point is usually so you're saying we should abandon code reviews or aha so you're saying we should abandon pull requests my answer is no to the first one and yes to the second i think that code reviews are important but often not done very well and if i'm honest i think that pull requests are one of the ways that they're not done well my impression is that some people can't imagine code review without pull request clearly this is wrong there are lots of ways of organizing code reviews so why are these things continuous integration and pull request or code review intention in the first place the problem is that if you practice continuous integration then you value fast clear feedback you want to get a definitive answer to the question does my change work multiple times a day that means that you must try it out checking to see if it works and if it works with everybody else's changes is that's the only way to get that definitive answer that's what continuous integration is really all about if you practice continuous delivery it's even worse than that now we need to know is my change releasable multiple times per day so we've just raised the bar on that definitive threshold for success as i said the data says this is the best way to work fastest and highest quality so that means that to achieve these results we'll need to compromise on some other things because this is how we build better software faster until somebody comes up with some counter evidence there's really no place to hide here the data says we need this feedback multiple times a day or at least once per day working in many small steps and getting a clear answer after each step works better than anything else that we know how to do it doesn't really matter if you like this idea or not that's what the data says so if we are to be rational that means we need to work in ways that deliver this level of feedback we need to optimize how we work to continually and consistently deliver this level of feedback that means that anything that prevents it is a problem the commonest barrier that teams build that reduces our ability to get to get this feedback is injecting more people into their feedback cycle testers security people code reviewers compliance people whoever the trouble is that people are slow we are clever and and our insights may be valuable but we are slow so let's imagine a classic pull request approach i make a change on a feature branch and i create a pull request asking you to take a look and approve my change you're probably busy and need to finish whatever it is that you're working on right now before you're ready to look at my pull request asking how long this takes is obviously an impossible question to answer minutes hours days i've seen teams where this can take weeks clearly those are dysfunctional themes let's be generous though and imagine that it takes 10 minutes that seems like a reasonably low number 10 minutes for you to wrap up your current train of thought and start looking at my pull request what do i do for that 10 minutes actually it's going to be longer than that 10 minutes to start and another 5 or 10 maybe for you to approve or reject the change so i'm now waiting for 15 or 20 minutes to see if my change is approved before it's even submitted to continuous integration in general when i'm coaching teams and organizations on how to adopt continuous delivery my advice is for them to aim for a result from continuous integration commit to results in under five minutes so now i'm going to wait 20 to 25 minutes for an answer to my question does my change work with everybody else's let's think about this from your perspective as the reviewer for a moment in continuous delivery and continuous integration we consciously aim to increase the rate that we get this feedback after all the data says that making progress in these small small steps is a hallmark of high quality code and delivery i practice test driven development so my natural cycle is red green refactor commit i write a test run it see it fail write some code run it make it pass refactor the code and test to make them great and that leaves us in a stable understood state so now all we need to know is does my change work with everyone else's so now is a great time to commit committing is going to give me that little endorphin rush from continuous integration knowing that my change is good and that's ideal at this point if i'm on a roll i'm probably committing to master or to trunk the definitive relatable version of my system every 10 or 15 minutes ignoring the time it takes to do a pull request for a moment if i'm asking you to process my pull requests every 10 or 15 minutes how long is it going to take before you're really annoyed with me it's simply not sustainable so we're going to need to take this human element into consideration too how often can i reasonably ask you to process my pull requests without annoying you again not a question that we can sensibly answer in general terms it probably depends on all sorts of things how many other people's pull requests you need to deal with for one but also how tolerant you are of idiots like me annoying you and asking you for requests all of the time but if you can't process my pull request at least once per day then by definition continuous integration is impossible so if you only have one person's changes to review mine then you need to as a an absolute minimum review one change per day for continuous integration to even be possible in reality what usually happens is that people store up their changes instead instead of me bugging you for a review every few minutes or even once per day i store up my change and only ask you to review them much more infrequently so continuous integration is now out of the window completely most teams that work this way operate develop development on feature branches that last more than a day and so make continuous integration by definition impossible but remember continuous integration is one of the key characteristics of high performing teams so now we're stuck in a destructive loop driven by the practicalities of code review then there are the reviews themselves 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 a really bad way to organize a development team if you can't trust your teammates to make changes carefully then your version control system is not going to fix that for you the real answer is to figure out how to make your colleagues more trustworthy that aside what about the quality of the review pull requests were designed so that people who were remote from one another and probably didn't even know one another could communicate it's slow an inefficient way to coordinate work but where there are where the alternatives are to either lock the doors or give every hacker free access it's very useful and an effective tool but it's slow and inefficient and we really don't want slow and inefficient processes inside our development team one of the features of a pull request driven approach is that it's asynchronous i issue a pull request you pick it up when you're free and you review my changes without necessarily talking to me i guess we could use the pull request as a signal for you to talk to me and i know some teams that do that but mostly you'll just do the review on your own so now we've lost two-way communications as part of the review maybe i had a good reason for doing the things that you hate in my code maybe you could use my mistake to teach me something important except now you can't because i'm not there so i prefer a conventional code review where we get together and have a two-way conversation about my code a pull request where there are where the only feedback is either changes to the code itself that i may not understand or agree with or a written description which is always lower bandwidth than a real interactive conversation so what can we do instead well there are a few options i've worked in teams where we added pre-commit hooks into our version control system that required every commit to be signed by two developers the coder and the reviewer i've seen this work but it's not that great to be honest it still stuff suffers from the problems that i described i'm going to annoy you really quickly if i'm asking you for a review every few minutes i've also built code review tools into deployment pipelines you couldn't release unless there was a review in the system for each change this was pretty horrible too but some in some ways rather enlightening because actually it was only as horrible as the pull request model it was just it surfaced that horror more clearly the problem in both of these cases is that you always have changes in one of three states the changes being worked on before they've been committed the changes that have been been submitted but not yet reviewed and changes that have been reviewed and so are ready for release in continuous delivery anyway the problem is that the changes are not yet reviewed they are in an indeterminate state clearly they aren't releasable yet so any definitive form of continuous integration is impossible yet again this is nothing to do with technology nothing to do with how i'd prefer to work it's just a fact we can't release unreviewed changes and we're struggling to find a way to review things fast enough to allow for continuous integration and continuous integration is the route to delivering high quality code fastest there's a simple solution well there's a solution anyway the simplest way to solve this problem is to include the code review as an ongoing part of the development process pair programming we work on changes together and that provides the second pair of eyes and the constant code review without slowing down the development process i am a big believer in pair programming for lots more reasons than only these still not everybody likes pair programming but the reality of my experience is that it works better than any of the alternatives building the review of our software into the act of creating it is the most efficient way to organize our work in terms of speed and accuracy of feedback i think that the pushbacks against pair programming are misguided but somewhat understandable organizations tend to worry about the seeming inefficiency of two people doing work of one the evidence though says that this is just a mistake pairs make progress at nearly but not quite twice the rate of an individual working alone but they also produce work with significantly fewer defects so overall they are more productive individual developers are usually more worried about this being an intrusion it makes them feel uncomfortable or they are nervous that it will disrupt their thinking the first concern is true for some people pair programming makes them feel uncomfortable my experience though is that this is a smaller proportion of people than you may think and what tends to be true is that people believe this more before they've tried pair programming than they have once they've experienced it most people that i have seen try pair programming actually prefer it as a way of working not everyone but most data on the second concern he's out there and mostly it says that pairs are actually able to sustain a better focus than individuals working alone and that reflects my personal experience too in some ways this is a big problem if we have a practice that works better but is unpopular amongst people that haven't tried it what do we do i'm afraid that i don't have a very good answer for that my advice is for you to give pair programming a chance if you haven't tried it before get your team to try it at least for a couple of weeks agree to do all work during that period in groups of two or more people and then get together to discuss what you found at the end i think that the results might surprise you thank you very much for watching [Music] you

Channel: Continuous Delivery

Views: 225,337

Keywords: pull request, don't do pull requests, what is a pull request, pull request tutorial, pull request issue, git, github, github pull request, create pull request, pair programming, ci pull request, cicd, continuous integration, devops pull request, computer science, continuous delivery, devops, Dave Farley, software development, software engineering, code review, code review github

Id: ASOSEiJCyEM

Length: 19min 12sec (1152 seconds)

Published: Wed Apr 06 2022