Pull requests are widely used. Most teams create a pull request somewhere on their route to releasing change. They see them as the only way to conduct code reviews, but they are not. Pull requests were invented to manage large, open-source projects, but working within an organisation with a team is not the same thing as open-source programming. In general, our advice is don’t do pull requests, but why is that, and what should you do instead?
In this episode, Dave Farley, author of Continuous Delivery and Modern Software Engineering explores the idea of pull requests and the issues that they raise. Dave offers several alternatives and discusses the pros and cons of each, ending up with some concrete advice on what you should do instead.
-------------------------------------------------------------------------------------
Also from Dave:
🎓 CD TRAINING COURSES
If you want to learn Continuous Delivery and DevOps skills, check out Dave Farley's courses
➡️ bit.ly/DFTraining
📧 Get a FREE guide "How to Organise Software Teams" by Dave Farley when you join our CD MAIL LIST 📧
The best way to keep in touch with the latest discussions, events and new training courses, get FREE guides and exclusive offers. ➡️ www.subscribepage.com/organis...
_____________________________________________________
🔗 LINKS:
DORA - State of DevOps ➡️ www.devops-research.com/resea...
DevOps Handbook ➡️ amzn.to/2LsoPmr
“Pair Programming - The Most Extreme XP Practice?" ➡️ www.davefarley.net/?p=214
“Pair Programming for Introverts” ➡️ www.davefarley.net/?p=267
“Pair programming improves flow” ➡️ journals.sagepub.com/doi/abs/...
‘The Case for Collaborative Programming’ Nosek’ ➡️ bit.ly/1J6LrAP
Strengthening The Case for Pair Programming’ Williams, Kessler, Cunningham & Jeffries ➡️ collaboration.csc.ncsu.edu/la...
‘Pair Programming is about Business Continuity‘ Dave Hounslow ➡️ bit.ly/1Pk0JFW
_____________________________________________________
📚 BOOKS:
🚨 📖 Dave’s NEW BOOK "Modern Software Engineering" is now available here ➡️ amzn.to/3DwdwT3
In this book, Dave brings together his ideas and proven techniques to describe a durable, coherent and foundational approach to effective software development, for programmers, managers and technical leads, at all levels of experience.
📖 "Continuous Delivery Pipelines" by Dave Farley
paperback ➡️ amzn.to/3gIULlA
ebook version ➡️ leanpub.com/cd-pipelines
📖 The original, award-winning "Continuous Delivery" book by Dave Farley and Jez Humble ➡️ amzn.to/2WxRYmx
NOTE: If you click on one of the Amazon Affiliate links and buy the book, Continuous Delivery Ltd. will get a small fee for the recommendation with NO increase in cost to you.
-------------------------------------------------------------------------------------
CHANNEL SPONSORS:
Equal Experts is a product software development consultancy with a network of over 1,000 experienced technology consultants globally. They increase the pace of innovation by using modern software engineering practices that embrace Continuous Delivery, Security, and Operability from the outset ➡️ bit.ly/3ASy8n0
Octopus are the makers of Octopus Deploy the single place for your team to manage releases, automate deployments, and automate the runbooks that keep your software operating. ➡️ octopus.com/
SpecFlow Behavior Driven Development for .NET SpecFlow helps teams bind automation to feature files and share the resulting examples as Living Documentation across the team and stakeholders. ➡️ go.specflow.org/dave_farley
From what I've read about pair programming, the studies, which are often fairly poor quality, have been generalised too much. Many of the studies (including the one linked in the description) is about junior developers or even students. It makes sense that working through a *challenging* problem collaboratively has benefits, to bounce ideas off each other, etc, but more senior developers will also do a lot of work that is within their comfort zone in their daily lives, and pair programming *all the time* seems just like a waste of resources to me and ends up being frustrating rather than inspiring. If you're a more senior developer who's doing work that's within both programmer's comfort zones, it just ends up with one person typing and the other staring at the screen, checking their phones or just zoning off. Pair programming is valuable if used selectively, that is, when two developers work on a challenging problem, or for a more junior developer to learn from a more senior one, or to occasionally get exposure to different ways of doing things even if it's within your comfort zone. A culture that encourages pair programming in circumstances where it makes sense is great; a culture that requires it even when it does not make sense is bad. I find statements like "always do" or "never do" to be dogmatic. Understand when certain practices are beneficial, and then apply them appropriately.
Spot on! I agree we should situationally use pair-programming to gain the best of both worlds! What would be a good explixit criteria when and when not to utilize pair-programming? (Afterall, such is needed otherwise people don’t know when and when not to do it.)
I definitely agree with you. I have only used pair-programming to get people run through some critical/big change of code, give feedback, align goals, etc. But it is always with the role of one person presenting a draft/PoC and the rest of the people commenting it live, leading to useful discussions that are often slower in PR messages. For day-to-day development, brains work very differently, tackle problems differently, have different speeds, need breaks at different times, organise themselves differently ... trying to get two brains sync up to do work is very inefficient, and ends up being one "brain" leading the path in its way, and the other "brain" trying to keep up in frustration (and maybe catching some eventual bug or giving useful feedback here and there). Not worth it 90% of the time IMHO.
I find pair programming much more draining than working on something myself. I reckon it highly depends on the kind of people in a team.
☝️This
@@squ94wk Agreed. It's not for me
I've done full time pair programming, solo go-into-a-cave programming, and everything in-between. I like being able to work across that spectrum rather than be stuck in one mode for too long. One criticism of pair programming I have from personal experience is that there are natural "breaks" that your brain wants to take as you're working, whether on simple problems or difficult ones. And, sure, yeah, that's when you check your email, or go get a coffee, or just get up and walk around. With pair programming, you often tend to "sync" up with your pair partner, which has its pros of course, but the con is that you're not taking mental breaks when you want to (and same goes for your pair partner), so you can end up really mentally drained after some time of it, maybe even without realizing it. And that's when you, yes even as a pair, start making some pretty stupid design decisions. The other issue is, of course, that some people work much better at different times of day. I am a night owl and figure out the trickiest problems after 11pm when I get into a "flow." With lots of people, that's not the case. So I think it's a really useful mode of work but I most liked doing it _some_ of the time. These days I don't code much at all, and I do kind of miss the camaraderie and shared gratification of pair programming.
Exactly my thoughts
...and most pair programming teams would agree, we fit the technique to the problem in front of us, this is about engineering, not about religious observance. Even on teams where pair programming was the norm, and the team thought of themselves as pairing all the time, there were times when we didn't because it made more sense not to!
@@ContinuousDelivery hiring should be based on buy one get 1 free perhaps?
I worked with Pivotal Labs where they rigidly followed these "best" practices to an extreme. The experience of pairing 8 hours a day also traumatized me. The entire project was a shit show, and it taught me that blindly and rigidly applying these practices, without understanding the context, culture and nature of work of the organization is a recipe for disaster. I might add I've worked on WATERFALL projects with better outcomes 😂
Blindly applying any practice is a recipe for disaster! :O
#storytime In one of my first jobs we had a "Sr. Consultant" working on another project offer advice, "the best thing you can do is do pair programming and ping pong TDD". Fast forward two months and "Sr. Consultant" asked, "how's it going?". We said, "good after we got over wanting to kill each other for being stuck together for 8 hrs a day". "Sr Consultant" laughed at us, "you lasted longer than I would have.... I meant, pair when you commit code but you should only be pairing half the day. Use the other half to gather requirements, do spikes, answer emails, etc....". The next two mo were great in comparison... I refer to this as my "pairing bootcamp". #lessonslearned thanks for attending my TEDTALK
I worked somewhere where pair programming all day was the norm. I agree, it was overbearing and made it hard to grow as an individual contributor. For junior developers, sure, that makes sense. But not for mid or senior engineers, to be pairing all the time without any room to execute at a speed that benefits them and the company most.
Certainly pair programming should not be 8 hours a day! That will be give me nightmares
I worked in a place where one of the teams went on mob programming. Not the best outcome
I understand the benefits and purpose of pair programming, but I find it to be a mentally draining process. It also impedes my ability to achieve a flow state of thought.
I agree. It is interesting the vastly divergent opinions on this topic. Perhaps this is due to the different ways peoples minds work. I posted a comment similar to yours on one of Dave's earlier videos and the responses suggested to me that many people have never experienced "flow." This is consistent with my observation that relatively few developers have the ability to sit and focus on a problem intently without distraction for 6+ hours without any breaks.
@@_winston_smith_ For me, after being in what I think others describe as a flow state, it usually ends up feeling like I lost my sense of focus. I end up with changes in more files than I can count on both hands and have no clue what to write as a commit message. It also seems I end up with less readable code that's harder to make sense of later on after getting out of that flow state. I actually like building in interrupts. For example: thinking about a commit message up front that would make sense to someone else or future me before starting on the code, preliminary rubber ducking, if you will.
I feel the same. I can't sit with a person on a call for hours on end. An occasional chat about an implementation? sure, but not a non-stop conversation.
@@_winston_smith_ for me, is the opposite; I mean that with pair programming, I avoid distraction all the time. Obviously, because there is another person who forces me to stay focused.
@@barberaf This makes sense to me. This is what I suspect is the case for the majority of developers and would explain the academic results mentioned in the video.
You can have builds running on PR’s which do a local merge and run tests on that. Ofc if there’s multiple PR’s/features at the same time you are not integrating anything and everything gets messy. But having tests run on a PR can give confidence of the quality of the change and assist reviewers.
Yep, we are using that approach for a few years and it works really nice. PR build using local merge is made, tests are run for it and additionally things are checked by QA team (both new feature and possible regressions). All is merged after they give approval .
CI isn't just running tests. It's integrating change. It's not full CI until it's in main. Edit: wait. Did I just reiterate what you said? My point is, tests on PR don't make it CI. Changes not being outside the mainline for too long is CI.
Run the full build and tests locally before every push! No need for branches.
That's just not practical for everyone. Our "full build" is over 200 jobs and takes hours to finish...
I think instead this video should have been titled "You need to stop creating large pull requests" and we could all go on with our day having short lived branches, fast semi-automated reviews, and approval by anyone on the team (including the author, if they so choose).
100% this. Have small PRs and short lived branches that are easy to review and have good test coverage.
100%
Code review is about readability, is about answering the question "will I be okay maintaining this?". If you're involved in the process of writing the new code (alone, in pair, mob, etc.), you're in no position of telling whether or not your code is readable, because you have fresh in your mind why you wrote it that way, the requirements, the failed attempts of writing iit another way, etc. The REAL readability test is when someone that doesn't have that knowledge swifts through your code and says "I understand it". Pair programming doesn't solve that. "One of the roles of a PR is to verify that someone who didn't write the new code can understand it. The constant communication of pair programming can result in code only that pair understands. Does a book with two authors not need an editor?" - Laurence Gonsalves
I always considered one of the benefits of code reviews & PRs is that it gets someone's brain who was not actively involved in the development process into the picture. Having a pair of eyes on the forest itself, that has not spent their time wandering through the detritus can be quite beneficial. But maybe, if the 2 dev's way of looking at things are different enough, this balances itself out and one or both are able to regularly step back for a grand view.
Yes, I have heard that often. In practice I have never seen a significant benefit as a result of the "distance". You get the same "distance" effect, only better IMO, when you include rotation in pairing. At LMAX, we used to rotate pairs every day. When the new person came in, they'd say things like "why did on Earth did you do it like that" and "have you thought of X". Then they went on to show why their idea was better, in the work, or decide it wasn't. I think this worked better than having someone dropped in after, notionally, the work is done.
@@ContinuousDelivery the benefit of having an "outsider" perspective in the process is that it ensures the documentation is also high quality. You can't guarantee that the two programmers who pair-programmed the code are available next time you need to change something; they might even have left the company. Having it reviewed by someone with a fresh perspective means that code and doc are clear enough for everyone to understand it.
@@brokenphysics6146 Well you could organise it so that you suffered these consequences, but I wouldn't advise it. My preference is pair programming with regular, frequent, pair rotation. When we built the financial exchange at LMAX, we rotated pairs every day, which meant that nearly everyone on the team saw something of nearly every feature that we built, and regularly, once a week, or so, worked with everyone else on the team. That meant that we never had a "bus problem" and never sufferance from siloed design.
@Broken Physics >the benefit of having an "outsider" perspective That's much less practicable than so many people think. The problem is that "outsider". Outsider means that they lack context. Having being outside does not help. It means that they haven't followed the development of that chunk of work, participated in the decisional process, nipped problems in the bud, steered development in the right direction. It's too late to give a fresh perspective. Most of the times, when faced with a chunky PR, reviewers have tied hands. They can only skim through it, capture minor problems with syntax, suggesting some refactoring here and there, and, in the best case, come up with a LGTM. Plus, the PR time is often too late to instigate a major rewrite of a feature, so what happens is that the reviewer suggests creating a tech debt ticket accepting to "leave it like that *for now*". Code should be self-explanatory, so if those programmers left, other programmers could pick it up.
@@andrealaforgia7699 if you're waiting with a pull request and code review until all is said and done and the entire application is comoleted, then yes, it's not useful. Instead, do it early, do if often. If your reviewers have tied hands when faced with a bad PR and can only suggest token changes, the problem doesn't lie with PRs but with your organization's culture and planning. Yes, they haven't followed the development process, that's exactly the point. They can spot problems the devs that are/were in the trenches can't see. It also forces the devs to justify their decisions and make pros and cons explicit. That way, you at least have info where your tech debt is, instead of being blind until it blows up in your face. Also self-explanatory Code is nice, but it can only ever explain the "what", never the "why". The latter one is a lot more important, though.
For me pair programming is extremely tiring. It's continuous communication, the direction of which switches sides periodically. Besides that, I just can't enjoy working while there is a person watching every LoC over my shoulder. Pairing / Teaming up to solve a specific problem fast and efficiently, in 1-2 hours, sure. But doing it full day every day? That's my nightmare right here.
It should not feel like overwatching but co-creating, building trust takes many steps and time. Pairing asks for more energy, it takes time to know when to stop, switching roles often helps, taking more breaks too. It's as if you were walking all day, but now you run, it goes faster but you can't run all day as you used to walk all day. Anyway, practices are a team matter and if some members are not up for it, forcing them won't work. PP, as any practice, is not for everyone.
@@angepicard7968 I did a lot of pp in my live (also continuously over month) the drain is real and for introverts a real problem. And your analogy to walking doesn't really work tbh. As an introvert all interaction drains energy and not like walking vs. running. And taking breaks is a good advice in general, but doesn't solve the introvert situation
Once again, pair programming does not mean "working while there is a person watching every LoC over your shoulder". That is NOT pairing.
My dream of a pair programming setup is more like what's thought of as what one might think of as permeable mobbing, and while it produces good code at a sustainable pace, it is less efficient in throughput than many working in parallel. I can pair up or join a mob for about 4 hours in a day before really needing a significant break (i'm an introvert), and generally can't do it more than about 3 times a week without some longer term impact. In the mob scenarios i've liked the most there were enough people participating that some could join and leave the mob to participate remotely (and independently) via version control and come back when rested, where the main mob was still functioning. What we'd often do on our own was carry tasks away from the main mob to finish them asynchronously (for example things like: speed up specific parts of the code, gather data for decisions that have been put off, refactor specific functionality (for example, switching or adding compatible xml parsers, fixing class hierarchy, making test fakes etc). i wonder if anyone else has had this experience.
@@prozacchiwawa Reminds me of the hackathon projects I was a part of. The ones which took place during several days, generally in the working hours (different for every team member). For me it was fun, but still only a temporary mobilization, with me feeling drained after a week.
I've been doing pair programming for the last 15 years. In my experience full time pair programming doesn't deliver on its promises. Intermittent pair programming (as in: at most a couple hours a day) works a lot better for me and I've seen teams using that approach be a lot more productive than dogmatic full time pair programming teams
I believe people in general can't or just don't want to focus for long hours when working for a company, period. We have our lives to care about.
I think it matters how you approach it, but I would certainly agree that it can be more tiring, it requires more focus for longer periods of time, but then working hard does. I have done pairing for a long time too, and mostly, in the teams that I worked on, it worked well for us for years, but I have seen places where it didn't work. I think that in those cases, it was usually down to two reasons. Their was a cultural problem in the team, of a few individuals who really hated pairing and so disrupted it. I confess, I really hate giving that kind of response - "It was you fault because you did it wrong" isn't good enough! In the places where it worked, I think that one of the reasons was that we consciously built a strong sense of "team" with other stuff beyond only pairing, and we rotated pairs frequently, so you were never stuck with the same person for long periods of time. Sorry to hear that it didn't work for your teams.
@@ContinuousDelivery I think that the domain in question makes a difference for how much pair programming is effective. I hear the story that "pair programming is great" mostly coming from backend and front-developers whose domains and tech stack are somewhat stable, and where exploration of concepts and implementations benefit from a constant discussion. I found that in math-heavy and/or algorithm-heavy domains such as videogame, machine learning, analytics, etc. having people talking over one's complex train of thoughts is generally counterproductive and actually leads to low quality software. The way I see it, the more complex the domain is, and the more it requires deep thinking, studying, discovery and reflection around a solution, the more the "full time" pair programming strategy is going to fail
@@sarqf212 Well, I have seen it work in derivatives trading, genome analysis and mass-spectrometer software, all widely regarded as different kinds of complex domain, so I am afraid that I don't think that that is the cause.
@@ContinuousDelivery Then my question for you would be: if you're trying to sort out a piece of mathematics, would you prefer to have someone talking to you all the time, or would you prefer to think on your own alone for a little while and then bring ideas together? I found that more often than not developers and researchers working in that sort of domain need time to think and study alone. I also think that the pair programming mantra assumes that building software is about cranking lines of code day in day out, when that's really not the case.
This appears more a reason to stop enforcing Pull Request *Reviews* than removing Pull Requests totally. Pull requests are a nice way to 1. enforce linking to a requirement/work item, 2. ensure that it builds, passes unit tests, and static analysis on another machine than the developers 3. act as a trigger to run integration or end-to-end tests in an on-demand disposable 'build/validation' environment Dave: I'd love to hear your thoughts on low-code platforms (e.g. Microsoft Power Platform) where the development loop is more convoluted and cannot be tested on a developer's machine.
You should see our power platform workflow...🤣. My first day on the job: "so wait, you tell me I download the artifacts, upload them to a temp instance, make a change, export the solution, then make a pr?" The entire cycle took about a couple hours just to change a small error! That said, canvas apps now directly support git and I bet the rest of power apps will someday. Also the tooling is there to make it better, it's just difficult to set up and doesn't remove all the pain points.
To your points though, I imagine he might say that you can use PRs but they shouldn't require review and they automatically merge if they pass basic tests?
The points remains, PRs are fundamentally against CI. Usually you can link testing to a precommit hook (altough I've never used MPP). If you're worried builds will fail outside of developer's machine, use a Docker container. Worst case, you want the source branch to break quickly so you can fix quickly.
@@Macknull dev's machine, docker container... Tell that to my embedded system where I have to do integration tests on 5 different hardware variants :D Those are of course automated via jenkins at some test-farms inside our factory => pull request to trigger and merge if green. CD on master branch.
Dave has already shared some of his thoughts on low-code platforms here kzhead.info/sun/qNx7iqqirGKDnas/bejne.html
Pair programming is great but I don't think it can replace PR reviews. Esp with a remote team in different time zones, its not sustainable enough to replace async PR reviews.
I mean, you talk about it like it's one or the other. You can have people do pair programming in the same time zone with other pairs in different time zones doing PR, and nothing really gets in the way...
And the argument against them is "the developer is sitting doing nothing while waiting for response". But a person can have multiple tasks and issues to resolve. So you can work on thing 1, commit and pr, then go to thing 2. So any reviewer has time to review.
The question here is if Dave's video can replace the need for PR. Answer: Click-bait can never do anything good. Do what is good for you, what your team helps to grow. Period:)
The first question is why you have a team with different timezones? Async collaboration will always have disadvantages over mostly sync collaboration
The general Problem is, that so called "PR reviews" are not really reviews. Usually people who doing a review just take a superficial look at the code if the code fulfills the general coding standards within the project / team. If you are lucky, the reviewer knows the code you are changing (because he wrote parts of it himself), than he might have a closer look and maybe give you a few suggestions. Most of the cases , the reviewer has never seen the part of the application before (especially in big applications) and just confirms your pull request without checking what the code is doing at all. This is especially true in companies where you often work under time pressure and everyone just wants to get his story finished within the sprint which the team has committed to. Instead of people talking directly to someone to do his pull review, they just commit a pull review to the whole team and just wait till someone confirms it without even talking to anyone directly and discuss the code. So pull requests are mostly useless and slow down development and contribute almost nothing to the code quality if people don't talk directly to a reviewer. Also the reviewers often don't know the Story or business case you are trying to solve and he also doesn't care about the story you are working on because he has other things to do himself than to understand the story / usecase you are working on. Pullrequests might be useful for new team members to get acquainted with the general coding standards within a project / team, but thats it.
You said that one of pull requests disadvantages is wait time for integration testing. Why wouldn't you do it on a feature branch? Our company is doing it all the time. What I understood is that you prefer merging everything straight to mainstream and review it later, but if such commit does not qualifies to project's standards or is wrong in a way or another, it can be hellish to revert it back, as other people adds code on top of it and starts depending on it.
Nice video with some very interesting points. I have been on the waiting side of a PR many a times and it is frustrating. However I do wish to emphasize one point about pair programming: it does not work for everyone. I know you mentioned this in your video, which was great, but I just want to bring awareness to one particular group of people with whom it most likely won't work: neurodivergents. I have ADHD and also some traits from the autism spectrum and for programming point of view this means that I have immense powers on concentration, but it requires a very strict set of parameters to make it work (no interruptions, music, freedom to work at my own pace). Pair programming pretty much disrupts all of those. I can do it in small bursts, but even just one entire day is a trial. I can see the benefits in pair programming and I'm sure it's a great way to work for many, but there are some of us out there that can't work with it, not because we don't want it to, but because our brains are wired differently.
I have worked with many neuro-divergent people over the years, some in teams that practiced pairing. To be honest it is my impression that there is a similar range of acceptance and rejection as for other groups as far as I can tell. The "normal" curve of acceptance rejection may be shifted, but there is still a distribution. It is certainly not as simple as all neuro-divergent people hate it, everyone else loves it.
@@ContinuousDelivery True, I generalized a tad too much, there will always be distribution. ADHD is actually split between the more familiar ADHD and ADD where ADD is less about hyper activity and more about concentration challenges. While it is ultimately depended on personal traits, I would still say that people with ADD are more likely to suffer from pair programming. I can't really speak for people with the hyperactive side of ADHD as my own experiences are more on ADD, but still my original intention was to bring awareness on the challenges pair programming can present to people with common ADD qualities.
I have ADHD and it's actually the opposite. My best grandiose prototypes come from 16 hour solo programming sprees but the quality is never up there. Discussing everything real time and bouncing off ideas at micro and more macro level keeps me engaged 100%. Otherwise I need that close to optimal environment to even start cranking at a feature. There hasn't been better method to keep me engaged and my brain stimulated as to thinking out loud with my colleague. Pairing allows me to consistently bring out my best abilities
@@13b78rug5h I have seen that too for some people. Thanks for sharing your experience.
Nice video! In our team we do a combination of pair programming and also have some rules that help us do pull requests-code review more effective, as: - Two daily slots to do Code Reviews 10-15 min each - Keep PRs below 200 lines of code - Follow the team norms/best practices for clean code - When a comment becomes a discussion get on a call with the author, it’s often easier to explain on a 5 min call instead of back and forth - When a comment requires a refactor, create tech debt ticket to address it and move on Also, we have learned that some code is candidate for a 15-30 min discussion session before start coding, taking this early feedback usually speeds up review, code/feature familiarity while keeping good velocity in change lead time.
Great CI systems will build your PR in a qa branch so you can see the results withing a couple of hours or you push your changes to a private copy of the repo and the CI system can build and deploy from a private repo. Once you have a build with test results you can do your PR with test results attached. Either way, you get immediate feedback on your change within the time it takes to do a build and for the test job to make it through the queue. So you can create a pull request and that will trigger your code to get built and run. Then if and when you find out your code works, then you request a review of your PR
Yes. This guy seems to sell the idea that CI/CD and trunk based development as the only workflow where regression tests are executed.
Yeah, it's strange to me he doesn't discuss this, as if merging to trunk is the only way of integrating. Heck, if you practice infra-as-code and use stuff like gitlabs review apps and merge trains, you can have all the feedback of technical integration including releasable artifacts as fast as you can make your pipelines. The technical part can be automated away until only the human feedback loop remains, and that is actually the problem that should be discussed.
@@LutgerBlijdestijn That would undermine his CI/CD narrative and selling point for his books. Why I cannot treat this guy seriously anymore.
Pull requests work very well for small teams. The problems of one person's code not working with another person's code - those just don't happen in small teams. Continuous integration addresses a scaling problem. The problem doesn't occur at small scales. And pull requests have other advantages which can enhance communication and make code review way way more convenient
On the other hand, if there's a super large team, than that's already pointing to the fact, that the software became too big and maybe it's a good time to think about splitting the work into multiple teams. Yes, I realise this is very often easier said than done.
Pair programming is like relationships. If it works it's great, if not it's a nightmare. The issue I have with pull requests is not that they're intrusive, but most of the time I have no idea about what the code I'm supposed to review does and what my colleague tried to achieve with that code. I guess we're a bit dysfunctional in our team...
Then you definitely lack planning/refining, which is another mechanism for improving quality of code (and also quality of product). Technical refinement sessions can compensate lack of code review, but also significantly improve alignment within the team and simplify PR reviews. The more predictable and easy to understand PR are the more motivated team members become to actually review them. In some (typical) canban setups with tasks created by a project manager not involving the team PP would be the only viable option of achieving consistent codebase.
Let me just say, thank you very much for all the valuable contents you produce always! After this, I've seen this video with a big smile on my face because it seemed me and you we've talked before because what you said is perfectly the same I say from a long time and I try to introduce and explain in companies and teams. Thanks again
I can recommend trying what we (former colleagues and I) coined "loosely coupled pair programming". The idea started at the tail end of covid. We had naturally transitioned to a remote first company, because even when we were not working from home where were in different offices across the country. So after our morning stand up meetings we would often stay in the voice/video chats as we begin our work. It became a method to have some social connection in the spatial isolation we had. We found that in this setting it became easy and convenient to do ad hoc reviews and mini discussions whenever anyone needed it. Over time we adapted the format to a kind of opt in pairing/mob sessions. Often when someone was working on a more critical or difficult feature they would ask for "loosely coupled pair programming", with anyone available. It's surprisingl not disruptive for the reviewer either, and it speeds up and improves both dev time and QA. You get a lot of the benefits of pair programming without needing two developers working on the same thing. I can also add that although we did merge requests, but we had no requirement of review before merging. Instead we had eventual reviews.
More people need to understand that pull requests are actually asynchronous pair programming if done properly. I find most people who don't like pull requests either don't see this or can't remove their ego from the equation and feel threatened by critique of their code. It is also difficult to multitask when pair programming; the asynchronicity of pull requests is what makes them useful in a scenario where multitasking is necessary to deliver.
"asynchronous pair programming" is just an attempt to redefine pair programming to fit some idea that pull requests must be. What do you think pair programming is? Just two people working on the same task?
@@ddanielsandberg "Pair programming" to me means working with someone looking over your shoulder making suggestions that could save you time and bugs *before* they get checked in, save your coworkers time and effort learning about the details of your work, and serve as a structured application of the rubber duck effect that comes from explaining what you're doing as you go. Pull requests "done properly" address all of that in a way that's more respectful of other developers' time and acts as long-term documentation of decisions made and feedback given. The rubber duck effect can also be replicated in a way that is more useful in the long-term with developer journalling. I would also point out that they don't require the continuous time-on-task and focus that traditional pair programming does and so are more accommodating of people who don't deal well with distractions/context switches or have focus problems eg. programmers with ADHD or autistic traits (quite a few of those out there!). Pair programming can be a useful tool (particularly for onboarding, in my experience), but it isn't a universal productivity boost for everyone and there are specific reasons for a dev or a team to prefer pull requests.
Thats not what pair programming should be. Thats not what it is at my workplace atleast. Where i work it’s more like one person writes a test, the other person writes code to pass that test. That person then writes a test and the first person writes code to pass that test and back and forth like that it goes. It works pretty well actually. It’s certainly not someone who just looks over your shoulder.
@@Ryosuke19 So what would you call what I described then? I'm using the definition that I've picked up through years of watching "the discourse" - from software development books, agile classes, Twitter, personal experience with management mandates etc. This is the definition I would expect my coworkers to understand if I used the term at work. If that isn't what pair programming is, then of course we are talking past each other, and if yours is the "correct" definition, then pair programming advocates have some work to do to change the definition in the wider discourse so they can actually be understood in these discussions. On top of this, I work in video games where test-driven development isn't a common practice and some devs are actively disdainful of it, so a "one dev adds test one dev makes it pass" workflow wouldn't be appropriate for my circumstances in any case. We do use automated tests, but there simply isn't a culture of automated testing ALL the things. Frequently our tools make doing that in a useful way difficult and we tend to have - and depend upon - armies of dedicated manual testers to find things that automation can't, anyway.
@@babgab Well i certainly would not call it pair programming if only one person is actually programming.
I found this video very disappointing, and is putting forward an approach that in my experience is not practical in most real world circumstances. As I see it, the only scenario in which eschewing PRs in favour of pair programming can work is for a single relatively small, highly experienced, team working on a product. If that's your circumstances then great - follow this advice. If you have a codebase that has dozens of developers of varying ability contributing though, I fail to see how this can be practical. The trunk will be a constantly moving target, the code that developers are working on will continually fall out of date, bringing with it the dangers and costs of merge conflicts with developers bringing their codebase up to date with main. Yes there are ways of minimising those risks, but generally only with careful coordination and planning in advance of changes being made. Overall, this feels like a nightmare to me. As others have pointed out, pair programming isn't a magic bullet to ensure high overall code quality or potential issues with bad actors. Two poor or mediocre developers aren't going to magically transform into a good developer. The main problems raised here with PRs seem to be around wasting time. In my experience, there are several ways to mitigate this. Firstly a developer who has raised a pull request should never just sit around waiting for reviews - they should move on to another task. CI should run on PR branches that are waiting to be merged, and CI tooling should ensure that there is sufficient test coverage, that coding standards are being is followed, etc. Ideally CI pipelines should be very fast, running tasks in parallel. PRs should be as small as practical, allowing them to be reviewed quickly, and team members should prioritise reviews.
Just discovered your channel. Loving this episode in particular. Really clear and insightful. Thank you!
Thanks, I hope you enjoy some of the other stuff here too.
I love this. I’m hooked on the channel. I work in medical and I am regulated. Pull Requests are required at every company I have worked for in the last 5 years. Because we didn’t do the code review if there isn’t an immutable document with a time stamp showing you did it. This has me thinking about all the glorious things I was able to do before I started working medical. And now I know I can never optimize for maximum efficiency and quality, because the quality requirements by the FDA promote auditability, not quality.
Thanks, I am pleased that you like my channel. What you describe is not actually in the regulations, it is how your company has interpreted them. I have worked with several clients in the medical sector, including on software systems that count as “medical devices”. You need a review, but there are other ways to accomplish this within the scope of the regulations. Pair-programming works fine, for example. You can meet most of the FDA regs within the scope of Continuous Delivery, in fact I’d argue that CD is the best way. At the top end, for medical devices that can kill people, there is a requirement for an external 3rd party review before release. That limits the frequency of release into clinical use settings, but doesn’t stop you working so that the system is always releasable. When I worked with Siemens Healthcare, they decided to release systems regularly into non-clinical (usually training hospital) settings. Gave them higher-quality, more regular feedback, and still worked within the regulations. I’d recommend looking at the regulations themselves, before ruling anything out.
I used to tell my boss to watch your videos cus they kept making me do feature branching. They watched your videos but didn't get it. Now I'm a boss and I tell my employees that they have to watch your videos cus they want to do feature branching. Please keep making content.
This has been a burning question of mine within the entire Continuous Delivery approach. Reading through the comments some similar concerns of mine are stated as well. One big one I find is: we currently do PRs and it is really beneficial having more than 2 people review the code that was changed and becomes a healthy group discussion. I do want to give it more of a shot though.
A strategy to this is “change review” at some regular cadence like at the end of a sprint. Key is that it shouldn’t be overly formal, a slotted time where a group says “hey this is what happened in the last week, what do we want to call out “ and let it naturally move to “wow that’s cool” or “why did you do it that way?”
@@ac.f.525 Yea we already do that, but it's more of an overall assessment of how our sprint went, if we decide that the way the pair implemented a feature is wrong, this is way too late to bring it up at this stage. When the PR is out there any developer can go in and actually look at the changes and provide feedback from whoever is on the team, rather than a bottle neck of two developers.
At my current team we've been using Upsource to generate automatic reviews of every commit made in the team, and everyone spends 30 min a day (either start or end of the day) looking through the commits others have made. This has massive benefits for keeping everyone in sync on what everyone else is working on, and gives IMO better reviews than PRs that tend to get one reviewer/approver. In theory everyone can look at a PR - in practice people don't do that. Sadly Jetbrains is discontinuing Upsource and I'm having trouble finding an alternative tool that will allow this workflow.
@@kasperstergaard1592 This sounds like an interesting approach, too bad about the discontinue. Thanks for sharing.
Thats all fun and rainbows when speed is the most important criteria. I can guarantee you that finance institutions value stability above all. PR help prevent people from pushing breaking code in production.
I think this point of view is the result of a very narrow application of pull requests. Like anything, it's a tool by which we can layer process and other tools onto to make effective and beneficial.
I agree. PRs are just another layer in the Swiss cheese model. They are there as a tool to benefit the quality of the software
Watching this critique of pull request model with the example of "speed" makes me think about one huge oversight - the "I" in the example also has to do the review. So while your coleague is checking your work, your checking theirs - no one is waiting a minute. It can sometimes happen with urgent out of sync changes but in general this is quite effective and fast way to work, especially while the pull request are small. As performed by Microsoft.
I've never tried pair programming but have done a lot of PCB layout pairing where I felt like an operator of the humanoid that obstructed me in using the software. If both people are similarly competent then it seems like a great idea. I would love to see a fly on the wall documentary about pair programming.
Just watched the video and I have a few thoughts about it. (long comment ahead) First things first : I do respect Dave A LOT. I learned about him only +/- 3 years ago and he became one of my references when talking about Software Engineering. I bought his Continuous Delivery book as well, read it and learned from it. This gentleman is a legend, period. However, even legends are not 100% right all the time. And I think that Dave is missing a few points on his analysis (and not only him, actually all the strong proposers* of Mob/Pairing + Zero-branching) The whole argument in the video is around "the code review overhead" and how it slows down the feedback cycle when achieved via PRs. That's fair, but it's built on premisses that are not detailed at any time, eg : (a) One HAS some other Engineer(s)* available to pair/mob (b) One can ALWAYS VERIFY that your changes are releasable from your local workstation (c) One NEVER MESSES with the trunk/main branch in the local workstation I won't discuss a lot about (a) because this is a non-technical problem : it relates to an organization and its Engineering culture. Sometimes one'll be lucky to have someone to pair, sometimes not (despite any personal preferences). My concerns are with (b) and (c). Imho they are quite ideal assumptions and although I don't have "data" to prove my point, I have plenty of counter examples to share from my experience (which is not not as extensive as Dave's, but at least built 100% on top of the VCS era). (1) Suppose one has a super slow build: the time to compile the code and run a few hundred the tests locally takes several minutes. However it takes only 1 minute to check things over the powerful CI machines; so one "games" the truth-first system by leveraging ONLY the CI pipeline to check changes, never running any expensive build checks locally, which increases the chances to have a forever-RED trunk branch; which means that we just defeated the purpose of Continuous Integration. (2) Suppose one does not need to compile anything, and running a couple of tests is super fast; but the project is also HUGE and we have thousands of tests in-place across all the features/teams/modules, which means a multi-minute execution for the entire test suite. This is the case for mono-repos in general, and again, non-branching (even short-lived ones) usually leads to the same situation as described in (1) (3) Suppose one has an awesome local workstation and works in a project with not that many tests; unfortunately only the CI machines can reach out to the staging environment where E2E tests will execute, due Security restrictions. So, one can never know about the release state of a change from a local workstation, since there is no guarantee of 100% flakiness-free E2E tests. (4) Suppose you have a JR Engineer in your team and he/she does his/her first "git rebase -i ever", dropping commits that were not supposed to be dropped in the process. Although the code introduced is perfect fine - when the local build proved that - eventually an entire feature just got deleted. Btw, if one ever force-pushed + messed a shared remote Git branch (trunk/main one or not) one knows the pain ... (5) Suppose you can't have in the local workstation the same environment you have in a CI machine, ie, your changes build great on your MacOS box but fail in the Linux one. Not that uncommon as it seems, when in a world where we have things like Docker. Remember that not all the Software produced in the world is a micro-service (6) Suppose that by "releasable product" we mean an artefact (or a couple of them) that is/are different from the ones that Engineers usually build in their local workstation; this is not super common in the back-end / micro-services world, but it is definitely the case for Mobile/Android and event FE Web, where code + assets will be transformed during a "production build" through a process that most Engineers skip when developing, since it is expensive to execute (but it is far from being bug-free). (7) Suppose that you have tests that look "stable" when running locally, but CI observability tools will shown only after a few executions that they are actually flaky. Now the entire team has a few more flaky tests to deal with locally, even if such tests have nothing to do with the current ongoing work/feature. Hence, one pair of Engineers committing + pushing perfectly valid code changes directly in the main/trunk branch accidentally slowed the whole team down. I could go on ... But I'll stop here, mentioning that all the examples above could be prevented with PRs + short-lived branches + CI-level automation. Eventually, what people hate about PRs are not the async code review at all, but the lacking of automation on top of PRs, eventually delegating to robots tasks that don't require attention from humans. I'd love to learn about Dave's thoughts on some of the points I've shared; meanwhile, thanks for the video 🙂
I have the impression, that you did not get the point of Continuous Delivery when you read it. The point is to still obey to some rules. Whoever does commit a change, that is not intended to fix or revert a breaking change, owes the whole Team at least a Coffee. Your scenarios all describe situations where you dont do CI or CD propperly. If you skip steps, because they take time, take some time to speed that part up.
I respectfully disagree here. Is privileged access over a pre-prod env. "not doing CI/CD properly"? Is the introduction of a flaky test in the test suite "not doing CI/CD properly"? Are slow build tools and/or slow test suites which force Engineers to spend hours per week validating changes locally "not doing CI/CD properly"? Is monorepo an anti-pattern for doing properly CI/CD ? Imho, no
The seven points you brought up all have one thing in common: They are feedback that something is wrong. You present them as problems when they are actually symptoms of cultural and technical issues. CI, CD, agile, etc also have one thing in common: To give everyone a healthy dose of reality and force you to do something about it. Flaky tests? Fix them! FFS! Slow build? Fix it, but don't fret over "a few minutes", you probably spend 10 times more time per day checking Twitter and getting coffee than a few minutes of building. People are force pushing? You allow that? Every SCM tool I've used supports to block force pushes (BitBucket, GitHub, GitLab, etc). Etc... sigh... I guess it feels easier to sweep the symptoms under the "rug of PR" instead of actually fixing things.
@@ubiratansoares Well, look at the comment of @Daniel Sandberg. I think he found some better description to what I wanted to say. -> Is the introduction of a flaky test in the test suite "not doing CI/CD properly"? The flacky test isn't the problem, the problem is to do nothing about it. -> Is privileged access over a pre-prod env. "not doing CI/CD properly"? If that means you do it only once every now and then and don't fix problems, or don't improve your environment to be able and do that tests more often, then yes you don't do CI/CD properly. ->Are slow build tools and/or slow test suites which force Engineers to spend hours per week validating changes locally "not doing CI/CD properly"? That is for sure not doing it propperly. If it is slow at the developers machine, it will be slow in your CI pipeline. Go back to Daves Book "Continuous Delivery" Chapter 1 and check "The Feedback Must Be Received as Soon as Possible" and also "feedback is no good unless it is acted upon"
>(a) One HAS some other Engineer(s)* available to pair/mob This is the premise of working in a team, isn't it? There is a widespread idea that a team is a set of individuals working independently and only occasionally catching up. I've heard many times the argument "helping the junior members of the team slows down the senior ones who cannot complete *their* work". That's completely wrong. There's no "my work", "your work", there's the "team's work", so get the members of your team pair up and mob cause THAT is the premise of teamwork. If that condition is not currently achievable, work hard to achieve it. My thoughts about (1) and (2): It's another work culture problem. If you have a super slow local build, that's a big problem. You need to address that. As a developer, you must be able to commit to trunk with confidence. The trunk build is not where you verify your local changes, so any developer who does that needs a good slap on their wrists. Any company that allows gaming the system that way has a big cultural problem that needs addressing. I am not going to comment any of your remaining points, because none of them really contradicts what Dave proposes or creates scenarios where what Dave proposes is inapplicable. Those are not an indication of situations where PRs are a better solution. They only show situations where CI and CD are either not being implemented properly or using a completely flawed toolchain. The fact that PRs, in those specific contexts, might come more handy because CI and CD are done badly, is not a merit of PRs. Sorry, but you missed the point.
Great video. Found your channel so now subscribed! The most common reason I’ve found for lack of adoption is that many individuals love to program individually so they can put the earphones on, crank up the music and get lost in the problem. Asking people to pair is asking them to give up what they fundamentally love about the job. What would be interesting is statistics from many teams who trial pair programming and singular programming + PR and be able to compare data to understand the actual gains. And by data, as an example I mean number of defects, cycle time, velocity, developer satisfaction. Measure everything. Being remote is also a challenge as “zoom fatigue” really comes into play here, it’s certainly a challenge for teams.
I am not a big fan of PRs, either, but I would like to comment on your definition of a rational argument. First of all, studies and statistics can only help you if you know all the variables that are relevant to your problem. If there are hidden variables controlling the output, this is a question that statistics alone cannot answer. I'll give an example in a field I know. For years, people were measuring the gap of indium nitride to be around 2 eV. At some point it was discovered that the simplest growth process for this material introduced defects, and these defects obscured all measurements. Eventually, the growth process was improved and the correct value of around 0.8 eV was obtained, which everyone agrees today. In short, I think your points in this issue are valid, but your general framework that just because someone made a survey of 5000 developers entitles them to put any policy just because it's the best we got, is very far from "rational". If we take this, anyone with significant amount of resources can impose anything, all they need to do is have more resources than the competition, nothing else would matter. Besides that, given the fact that large companies routinely lose leadership to a smaller competitor, I would guess that their studies are pretty far from recognizing the right variables.
I came to the comments to make this same point. Correlation is not causation. And 5000 isn't really that big of a number... bet if you stratified the results a bit by problem domain etc I bet you could find other candidate explanatory variables.
Also a survey result means little not seeing what the selection method was and what the questions and presentation of the survey was. Like if I go around asking "want to take a survey on why PRs suck?" You'd get a lot of respondents that think PRs suck, even if they aren't representative of the whole. And of course, people can hate PRs while not having a better solution.
>First of all, studies and statistics can only help you if you know all the variables that are relevant to your problem. The book "Accelerate" amply explains the foundation of the DORA research. We know the foundations of those studies very well. >just because someone made a survey of 5000 developers entitles them to put any policy Not sure what you mean by "put any policy". I don't think anyone who has conducted the studies mentioned by Dave is going to dictate aa way of working for everybody. On the contrary, I think what Dave says is indeed very rational. Isn't "where is your data?" the classic question to whoever is presenting an argument? It's called burden of proof. If I say that trunk-based development is highly successful, don't you want to see the data proving that? It's not about imposing anything. One is free to adopt whichever way of working fits their company/teams, but knowing what has been analysed in the industry is important. Knowing that many companies developing products very similar to yours are adopting a way of working that has proven highly successful for them, can be very valuable information for you.
@@thekwoka4707 >Also a survey result means little not seeing what the selection method was and what the questions and presentation of the survey was. Read "Accelerate". It's explained. >Like if I go around asking "want to take a survey on why PRs suck?" You'd get a lot of respondents that think PRs suck, even if they aren't representative of the whole. Sure, but that's not the type of surveys Dave is talking about.
@@andrealaforgia7699 Hello, thanks for the reply. I have seen the DORA metrics and I don't see any conceptual difference between them and any other productivity metric that have been put forward in the last decades. To give an example of statistical studies that work, polling for election results predicts the outcome pretty well. However, what not many people know is that pollsters are not going to the street asking random people. They did some studies beforehand and know a priori that factors like gender, income, region of residence, etc., have an influence on who you are going to vote, and then choose people in these demographics in a ratio that represents the overall electorate. It's well documented that if you just go out and ask people you get biased samples and cannot predict the outcome. These studies are pretty much the same thing. They don't do any but the most basic profiling of the development teams and simply measure the result. Again, it's well documented that this doesn't work. It doesn't matter if your sample size is larger than the rest. Also, I think it's totally fair for someone to say to another "show me the data", but another one is to claim that just showing the data is enough, which it certainly isn't. If you want to use this as an argument, you need to go through all scientific rigor. And the statistics applied to social fields like this one has gotten a lot of heat recently because no one seems to be able to reproduce their studies.
I usually enjoy Dave's videos, but this one was a real miss. Trunk-based development does mean that you cannot have branches or PRs. It's a counter-measure against long-lived branches. The reports mentioned in this video do not pertain to PRs, PR reviews, or branches. From the trunk-based development website: "Depending on the team size and the rate of commits, short-lived feature branches are used for code-review and build checking (CI), but not artifact creation or publication, to happen before commits land in the trunk for other developers to depend on. Such branches allow developers to engage in eager and continuous code review of contributions before their code is integrated into the trunk. Very small teams may commit direct to the trunk." From the State of DevOps report: "Our research has consistently shown that high performing organizations are more likely to have implemented trunk-based development, in which developers work in small batches and merge their work into a shared trunk frequently." The State of DevOps report then says that developers should merge at least once a day. And so, trunk-based development does not mean "directly committing every change to the main branch". It means that you a) can't have a long-lived feature branch and b) absolutely do not have an environment branch like 'dev'. The first point about long-lived feature branches forces developers to deliver features in chunks, which is excellent for CD.
Not really, CI is "commit at least once per day" by definition. The State of DevOps report says "if you integrate your changes at least once per day..." so it is explicitly describing CI. TBD is just another name for CI, but even if you don't agree with that, I was talking about CI - "merge at least once per day". If you do that, you can do pull-requests and branches if you really want to, but what do they add? I'd rather save work rather than invent it for no good reason, so dump the PRs and feature branches.
The primary meaning of trunk-based development is that you commit straight to the main branch. That "trunk-based" would not make sense otherwise. It's the same meaning of Continuous Integration, and why I consider the two synonyms. The meaning is extended to include VERY short-lived branches. Let's remember that the 2 days that Paul Hammant mentions are a max age for a branch and definitely an edge case. Realistically, teams working on short-lived branches create several a day and merge them multiple times into trunk. That's how I've worked in my teams where we were continuously integrating. Short-lived branches are typically used in contexts where you are not applying pair or mob programming and need to get someone to quickly review your code before merging, but in the case where social programming practices are applied, async reviews and PRs are totally unnecessary. PRs were devised in the context of open source projects, where you have hundreds of untrusted contributors who may come and go. That's not the case of consolidated, durable product engineering teams of trusted developers.
Excellent video, many thanks. I talked our team leader a while ago about going pair programming when I saw our PP video a while ago... It really does work and makes a great difference. Now our small 5 man team has been merged into a larger team of another product of about 20 developers, split into 3 sub teams, it's a nightmare now trying to convince the other 2 sub teams to change... because the overall leader of all three is just a xxxxx. Oh well. there lost.
I worked on a team of three that developed in the typical manner of individual coding with PRs for about three months, and then switched to mob-programming exclusively for three months, and after a short transition period mob-programming was more productive and more fun. Code quality also increased, as well as our shared understanding of the system and its design. As new people came on the team we continued to (almost) exclusively mob. I've moved onto another project but that team continues mobbing and seeing the benefits. There's and old anecdote, I think from Kent Beck, saying he and a colleague would pair on each others work and then head out early because they were both done with their days work. I've seen that kind of productivity gain personally and whenever I have the choice I would prefer to mob or pair with others who are willing than sit in a hole and code alone.
I agree with what you're saying and that pair programming with continuous delivery is what teams should strive for, but I also don't think it is realistic or possible for a lot of organizations and developers. I work at a research institution and participate in open source Python software a lot. There are simply not enough developers for how many separate research projects are being worked on. We know that setting aside time for pair programming or sprints can be really productive for the project, but it also means taking developer time away from other projects or closing developers off from people who want to consult with them on yet another project. How does CD work for released open source software? For an application (web or desktop) that is centrally accessed in an organization, CD is obvious. In open source packages you tend to see released versions. We may strive to have as many people using trunk/main as possible, but again the limited number of developers may not be using that project at this minute/hour/day/week. Or the users (and developers) would like semantic versioning so they know where/when they can expect API or other backwards incompatible changes. I just don't think there are enough people with enough time to always being using the trunk/main branch of a project like pandas when the results of using the software are the "important" part of their job, not keeping up with changes from upstream dependencies. I'm sure I'm missing something here, but I feel like this is a major piece I'm missing from your videos.
Agee, had same thoughts and pushed that to in comment section 😜
When you say "there are simply not enough developers" that indicates an assumption that pairing is going to take longer. If the statistics are true that pairing makes the work go faster (and that matches my experience) then saying there aren't enough developers is not a good excuse. Pair everyone up, reduce the WiP, and you'll see flow increase (reduced cycle time).
@BementalSea I specifically didn't say that. The problem is that that other developer I need to pair up with has a deadline for another project that I'm not a part of. When they're done with that project, I'll probably be working on another one. It is too many projects that is the problem, not the time it takes to do the programming (although that often takes longer than expected too).
@@djhoese Sorry if I misunderstood your comment. If too many projects is the problem, then you probably know the solution for that. (Organizations that think working on lots of projects at the same time is productive are kidding themselves. Kanban shows us that if you reduce WiP, flow increases).
@@BementalSea Yeah. I think people generally know this and I've emphasized the point with higher ups, but for scientific research it is hard to have separate "experts" work on each separate project when those projects are using grant money for a year or two and may not be funded afterward. I'm not sure there is an easy solution without requiring high-level management control all projects from separate funding sources with separate team leads that all want to do their own thing. "Ok these 2 weeks developers A, B, and C are going to work on project 1. Sorry project 2, we don't have any developers for your project right now."
I like what my current team does. We don't do pair programming, exactly, but we break the overall system into separately deployable major parts and we break the major parts into minor parts and we dedicate a pair of devs to each major part. So my partner in crime and I don't work on the same minor part at the same time, so we're able to work in parallel without stepping on each other. But we're both up to speed on the major part we're together responsible for, so if one of us goes on vacation or something, the other can stand in. Whenever we run into a snag, or aren't sure about the best way to approach something, or just want some confirmation that what we're doing is good, we get into a call on Teams and share our screen and go over the relevant code. We don't do pull requests, we have a repository for each major (independently deployable) part, and each pair of devs works in "trunk" for that part. We release first to a qa environment, then to user acceptance testing environment, then to production. When we deploy a release (a defined set of fixes and changes) into user acceptance testing, we create a release candidate branch, so that we can continue doing development in trunk for the next release while testing is going on, and then do break fix work on the release candidate and merge those changes back into trunk. QA and dev reps sit in defect triage and requirements review meetings with reps from the users, and put defects and change requests into Jira. Devs pick them up and work on them and deploy them into qa and update the tickets. If we need clarification on a defect or a change, we get into a Teams call with the person who put the ticket in. Occasionally it's determined that a defect is not actually a defect or a change isn't actually practical, for one reason or another, but the people who are putting in tickets are pretty technical and they understand the business pretty well and they're fairly experienced in general, so, most of the time the tickets are fine. I feel like this system works pretty well. I don't have anyone hovering over my shoulder all the time, but I do have a partner who is responsible for the same part of the system that I'm responsible for, so we're always updating each other and conferring with each other. I do get breaks quite often, when I'm all caught up, which I suppose could be said to be kind of inefficient, at times, but I think it's actually a really good thing, because it provides a genuinely appreciable reward for getting stuff done in a timely way and it really helps avoid burn out.
So this outlines that when you do Pull-requests in the way he outlines them, they are bad. But a Pull-request is just a feature of a tool, no one says that you have to have a manual review process in there if you don't like it. PR's is a tool that can help us to see if you code can be integrated into the trunk/main/master (whatever you wan't to call it) without breaking anything. You can kick off all your automated testing and statically review efforts on a PR without committing the changes to master and break everything. And it has the advantage that it's IMO easier to parallelize this process allowing for faster feedback than if you just ran all those tests locally. If it all passes, you merge the PR and DONE. Also, how many times have we not heard (but it works on my machine)... When you see PR's only as a tool to embed a manual review process, and can't possibly see any other uses for it, then your honestly extremely narrow minded and should perhaps not speak on the subject.
I was skeptical of how this knot of constraints would be untangled but the moment I heard it would be pair programming I punched out. Even before the pandemic I mostly worked with my colleagues remotely with only part of the day overlapping. Now we rarely even attend our respective offices, and in future we hope to be fully remote. We quite often work on entirely different services in parallel though in pursuit of the same overarching goals. If we had to coordinate time to pair program one change at a time we would add a ton of constraints and friction to our work, while still needing a backup plan for when some changes had to be done while nobody was available to pair on it. If we have to fall back to code reviews for those changes anyway, I'd argue we're not much better off in the metrics you're promoting. The compromise we've found thus far is to never stray too far from main. We have "feature" branches that have only 1 commit each (so that PRs can be created), as small and focused as is practical, and to be reviewed within one business day at most. This has let us work semi-independently while never straying far from what other people are doing and never leaving code unreviewed or unmerged for long. The worst thing that happens here is that sometimes changes stack up on top of each other and have increased costs to rebase and eventually merge, but honestly I would still take that cost occasionally over the cost of pair programming for the majority of my work. It may not maximize all the metrics but it's a lot more flexible and accommodating in today's remote-first environment. I'm sure most would agree that falling back to alternative working styles is better than not being able to proceed at all for most hours of most days.
Okay, so you work on service A and your colleague works on service B. How would your throughput change if both of you worked on service A on Monday, and both of you worked on service B on Tuesday?
@@mrlordbrutish it would plummet. Obviously. Because focus switching is widely known to be provably bad, and pair programming is inefficient. You can't fix inefficiency by adding more inefficiency.
@@mrlordbrutish I’m not sure what you’re driving at here, since it appears that you are challenging the op’s viewpoint of being anti pair programming (PP), but the scenario you gave would never come out in favour of PP. Assume the work for services A & B both takes 1 day each, and both devs work at the same pace. Working independently the work for both services gets done on Monday, leaving them to start other work in the Tuesday. Clearly the throughput is worse. In fact it would probably take them longer than two days to deliver the work for A&B since there would likely be discussions about the best way to implement the feature, which wouldn’t happen working independently. Now those discussions may be ultimately beneficial, but they can also hinder progress. I think the best use case for PP is a mentor-junior relationship where they can work through the implementation together, which if implemented independently by the junior may result in a lot of to and fro in the PR.
I love when Dave confronts the status quo and brings new ideas that are not "new" but very consolidated.
Confronting the status quo as a means of becoming relevant…these ideas might work in an ideal world and for ideal projects. A code review costs what 10-20% more work hours? Well guess what… pair programming costs 100% more. In some cases pair programming can help, but as a means of knowledge sharing while also producing a feature for example.
@@pgeorgiadis2 Wait, what makes you think that pair programming costs 100% more?
@@pgeorgiadis2 Ahhh, the good ol' "in an ideal world" argument. These practices work in the only world we know. There is data showing that they work. It's a bit stupid to question other people's experience on this. Pair programming does not cost 100% more. Software engineering is not bricklaying.
@@pgeorgiadis2 Have you tried it? What can you share about the experience?
Hi Dave, great video. I would love to try this with my team but we're quite globally distributed. Of the 8 of us or so I think only two are in the same timezone. We struggle to find time to do bi-weekly sprint meetings. Would you recommend a reorganization of teams via timezone? And if not, should we just work on our own until there is overlap between two members? Lastly, I have a few members who have made it VERY clear they don't like pair programming. As someone less experienced and newer to the team how I could I nudge them to try this?
I think the main reason he is suggesting pair programming is as a way to make your teammmates more "trust-worthy" in committing code that follows your standards. This way, TDD can take over and eliminate excessive code review. If your teammates write code that follows the project specifications, and also fits the CD/CI requirements (passes all the tests), then there is no real need for pair programming or excessive code review--so long as your tests are written to catch all bugs and test production quality suitably. I think the nudge in your case should be toward exceptional documentation of standards. Let's say the consensus is that 'for loops' are superior to 'list comprehension' in your case, so your boss will want to document this in your team's standards. If you fail to follow the standards, your code will still pass the production-ready tests, but not pass the internal standards. This can be tested for as well, however. This is just one possible alternative, but I also prefer it to pair-programming (at least at first) since setting written/documented standards is easier if done before pair programming starts. You'll need to remind teammates less often to use that 'for loop' since they've already learned the standards.
I was looking forward to this video! Thanks for your insight Dave. This is definitely going to be a controversial topic.
Code Review for me is a mandatory human element for merging. And CI is the one that complements that and reduces the load from the reviewer to not deal with mundane stuff like formatting, lints, tests. Code review should only happen after a successful CI run. CI even with a 100% test code coverage and the most strict checks cannot guarantee that you wont push something inefficient or non-compliant to the business request. People make mistakes, the human element can't be thrown out. PR's are just a tool for that, and can be used in many ways. What I'm saying is PR's are not in the blame here but bad management is. An interesting review process that kinda falls in line with your pair programming is defence. Like when you wrre defending your thesis. A review could go toghether where the author explains their code instead of the reviewer trying to figure it out themselves.
Yes, all the tests in the world won’t protect you against unhelpful variable names, stale comments etc. CI can be done at the same time as a PR I don’t see it as the blocker being touted here in my experience
The mistake is thinking that PRs are needed for code reviews. Code reviews can be done continuously. No need for PRs. PRs were never meant for durable product teams of colocated, trusted collaborators. They were meant for open source projects having lots of external, untrusted collaborators. Peer review == pull request, as Thoughtworks also says int their tech radar, is a false equivalence.
I agree with all the points in the vid, and I'd like to add some thoughts to the pair programming: My experience is that for tasks that require me to make an extensive mental model of the problem I'm solving (e.g. chasing down a weird bug, sifting through tonnes of data, or something heavilly mathematical) someone talking next to me (or "thinking with me") really impedes my train of thought, to the point that everything in my head just blocks. For 95% of writing code, pair programming is not a problem. But the really tough problems I prefer to do alone in a quiet place and have reviewed afterwards. Also don't underestimate the vast differences in mental models. I have mostly worked in the high tech industry (so no continuous delivery) with for example data scientists, and for most of them every data structure is an n-dimensional-matrix. Or audio/DSP-specialists for whom everything is a buffer, image processing scientists where everything is a 2D/3D image. Every expertise has their own way of mapping their domain/mental-model onto software (sometimes for good reasons even), and combining that with what a software engineer considers "proper software" can be a real challenge.
I personally like the approach Sourcehut uses. It basically emulates the "original" way of using git (with emails) runs CI against every Patchset before people review it. And review happens at the contributors availability, without pressure or interruption of work.
Yet again a suspiciously well timed video for my current situation.
Trunk based development is always best, however we need to have a continues review (i.e after the code is in trunk) and unit tests that run too fast and when most in the team are senior enough. It also reduce conflict and merge issue than and there.
Like with everything else, treat this video as a hypothesis. Go form an experiment and draw your own conclusions as a team. If you're just changing things Willy nilly because you saw it on this channel or some Twitter thread, you're no better than the executives who see buzzwords in airplane chair back magazines and come enforce mandates that provides zero value to the organization besides padding their own ego
Connecting PRs with a review is really stupid. I haven't seen this work at all. It has always been a means to enforce low performance which weirdly seemed to be desired because those project managers were control freaks. I'm not against PRs to remotely run the CI/CD pipeline in the background for me. It can come in handy when developing e.g. outside in the garden at a small laptop. PS: thanks Dave, you're the best, inspired me a lot
I am so glad to hear this. I got stuck in pull request hell where the main dev kept delaying my check ins for a week until the trunk had moved on and my changes were no longer compatible. Over and over. Pointless thrashing.
Thank. You. I’ve never seen the benefits over delaying reviews over doing the reviews when we do the code.
I would really like to see data whether this continuous integration works in places were you cannot do small changes that last only one day, or were testing is a procedure that is extremely challenging and thus cannot be done too often on the threat of bankruptcy. Ie. the american airplane F-35 program - they tried "fast" development which for them meant one release per half a year - and they experienced only increased amount of errors in comparison to previous way of doing a release per few years (or basically when they felt that they were ready for it). So they went to 1 year per release because they still wanted to be "agile" and make "frequent" releases, but noticed that half a year per release was a step in the wrong direction in terms of efficiency. Of course, you cannot test a new release of F-35 software every day, it is simply unfeasible no matter how you look at it. There are many economy branches were software isn't some simple application on a website and in most of those branches testing the software is a complicated and expensive procedure, while errors can be deadly. One should know about statistics a very important thing - the majority overwhelm the minority. In IT the small web applications are the majority. So can it surprise anyone that small scale approaches win in statistics? Remember, on average we are all chinese ;)
Look into how Tesla and SpaceX work. Tesla recently upgraded their factory to increase the max charge-rate in Model 3 cars from 200 Kw to 250. This was a minor physical change in the design of the car. It took 3 hours before the factory was building the new car, and that was because of these techniques. I worked on a team that built one of the highest-performance financial exchanges in the world, and we worked this way. So it does work, but it takes a big change in mind-set.
@@ContinuousDelivery of course, it would be foolish of me to ignore what you say and I don't want to do that. When you say financial exchanges, I know what this means, because I work in finances as well, in financial world you cannot make an error that is as large as one cent, but there are many potential places were you can make it because in many places you need rounding, not mentioning the numerical rounding that is automatically done if you keep your money amounts as floats. Though my mind-set is different then Yours. My mind-set tells me - don't assume everything is the same everywhere and also - don't assume that there are no circumstances were generally approved theories don't work.
Tesla is a bad example as they are notorious for poor build quality - they're ranked 1 before last by Consumer Reports for reliability.
The F-35 program is not an example of "working fast" (or rather CD/CI). It's an example of still trying to do things the old way but force more rapid releases despite the workflow still being PRs/waterfall/manually handheld pipelines. This will always lead to more errors are you merely force less control on a faulty workflow. The point is to change the workflow to support CI/CD. There is a niche of software development where PRs and "slow" handheld pipelines and releases is appropriate - and that's the "No errors are ever acceptable" niche. This includes banking and finance since you are risking other people's money, but probably doesn't include F-35 development since SpaceX has shown that even the yet more expensive Rocket development is better off with CI/CD and fail fast.
@@ContinuousDelivery if something goes wrong though in either of those situations people don't die though. Having worked in a defense project there are multiple reviews, sign-offs, simulator testing, and love fire tests. You literally can't do that every release. We did do fairly regular builds and releases to a simulation environment.
We use git flow, so each feature or bugfix associated to a Jira ticket is a branch. Everyone can deploy any branch at any time on development server. A pull request is only required when a branch is complete and can be merged into a release branch. A merge conflict may occur with previously merged pull requests from other features. As a result a pull request is not a way to check that "my work works with other's", this is the goal of the release branch. A change that has been reviewed is not a guarantee for release : only a thorough testing of the release branch can detect regressions coming from incompatible changes.
I might be missing knowledge about that but what kind of test to you trigger when doing a PR, I think unit tests and code style are a minimum but to reach automatically approved PR, I think I would need a dependency check (With a verification of the support behind a dependency, got someone trying to push not so used dependencies), a tool to check the absence of credentials/password/api keys or hard coded access-ish information in the code, automated penetration tests... In a JavaScript world, what tools would you recommend for that?
Feature branches can be continuously integrated with either manual or automatic merges (or rebases) into it from the master branch. Then just a pull request (or 2) to merge the feature into master and then closing the feature branch. Doing that I believe negates the problem of multiple out of date pull requests. Pair programming might be more fun depending on who you are paired with, but I need to get into ‘ the zone’ when coding so pair programming is not for me.
CI is not just merging. CI is a human practice and it's not CI unless it's merged INTO master several times per day. Can everyone just stop redefining CI to be "whatever we're doing with branches right now"!?
I feel like the majority of this guy's videos just assume that nobody knows how to use git properly. And also that he doesn't know how to use git.
Here's a thought: Why not schedule one or two "PR review sessions" each day, of 30 min to 1 hour each? In these sessions everyone would do PR reviews, and can ask everyone else about their PR's (to resolve discussions quickly back-forth without prolonged textual discussions in the PR). That would enable CI ("merge at least once per day"), and give predictability of when your PR will get reviewed, as well as limiting the interruption of people's work / flow-state, by batching the PR review work. It also would limit the buildup of unreviewed feature branches and limit merge commit issues.
Once a day though is a pretty low bar - a minimum. When I'm pairing/mobbing and doing TDD, we are committing every 5 or 10 minutes. A once a day "PR review session" isn't going to be able to keep up with that pace of change.
@@BementalSea I agree, one is a minimum, that's why I also said "or two". If you require PR review every 5 or 10 minutes, won't you be disturbing your coworker's "deep work" sessions?
@@magne6049 Yes, even twice a day is not enough for teams practicing continuous integration/delivery. And yes, a pull request every 5 or 10 minutes is too disruptive - it would never work. The recommendation here is to eliminate the pull request, and replace it with pairing or mobbing. I've had wonderful experiences working in this way. With pairing or mobbing, pull requests are not needed, because multiple people have been continuously reviewing the code as it is being written (this results in higher quality code, compared to a code review, which is inspecting after the fact). This is in the spirit of eXtreme Programming - if something is good, let's do it all the time. Meaning, if code reviews are good, then let's do them constantly (i.e. pair or mob program). Regarding "deep work" sessions, or being in a state of flow, I'll also mention that flow is achievable when pairing, and even more so when mobbing. (It doesn't only occur as a solo developer).
Excellent, this guy said what I had in mind for long time and never spoke out.
Is this model expected for Ops as well? I do IaC and have been struggling to find a way to optimize trunking concept with Ops focused code delivery. It could be that I haven't matured the env to using true pipelines yet are it is ran ad-hoc and maybe it will make sense then.
From my experience, pair programming allows one of the developers to just fade away, specially when doing it remotely via teams or similar tool. It is quite annoying to realize that your teammate is checking his phone while you are actually doing something.
Pair programming is still a work in progress
You certainly have to work at it. My preference, as well as pairing, is to rotate pairs often, so that you don't get stuck with working with people who you don't like working with all the time, and you get to learn from everyone on the team. Still, part of pairing is to reinforce the other people, so if people are on the phone, try and find a way to remind them that they are working, or should be. Maybe say "I see you are on the phone, want to take a break for a few minutes, then we can get back together and concentrate".
I had the opposite experience, that people don't want to appear to be slacking off so try harder to resist distraction. I only tried it in-person though.
@@edwardharman1153 That is certainly closer to my experience too.
I had this problem too. I think it really comes down to focus and having a teammate that you are up front with. I think it extends beyond a problem with pair programming but an issue with someone not being fully engaged or maybe not respecting the partnership of pair programming
The organization I work in (US, retail sector, e-commerce dept) is currently staffed very aggressively with a large, almost completely off-shore contractor pool. (A fairly well known one, not that the name is material.) It is without exaggeration that the few company devs are outnumbered 20:1 or more by our contractor counterparts. Due to the extreme time difference, our overlap with our counterparts is about an hour or two per day, during which too many stand-up and technical calls are trying to happen just to try to stay in sync. Communication is no one's strong suit. All that to say: the idea of pair programming -- something I have not experienced in my 20+ years of coding professionally -- just cannot happen in this environment. And while Dave did not dwell on this point, we do have an issue of "trust" in the quality and professionalism of the code we get. To that end, we have various pre-commit and pre-push hooks (which poor actors seem to know how to work around), pull requests requiring two approvers (all of which are also off-shore counterparts while we are sleeping), and other things. We have recently began introducing in-pipeline quality checks ... which, granted, should have been in place from the beginning, but they weren't. Yes, the 2-approve pull request cycle slows things down, and for all the reasons Dave mentioned. I have probably 2 or 3 other devs in my time zone available on an average day, and they're busy fighting their own fights. Naturally, the bigger issue we are facing is just the composition and geographic spread of our resources -- in effective, the few devs working in the building are rather the offshore crew of our own product. But that's something management has made pretty clear will not soon be changed. So we use some of the tools and human processes in a rather more defensive posture. (At least the pay/benefits/bonus is good.)
Sshhh, religious followers of this channel will quickly tell you that you did not understand the video.
As someone else mentioned, pull requests allows asynchronous work. Also i as the person refuse to work in the 9 to 5 window to be able to pair code with anyone else every day/full day and thus do not use pull requests..
If you think the causal factors that make CI come out top in industry surveys trump what's going on where you work, you are insane. Every company I have worked for is dysfunctional in their own beautiful way: - Bob is the longest standing developer with the most knowledge. He hates pair programming and unit testing - The guy that wrote most of the system left 2 months ago - All changes must be signed off by QA. There's one tester and they work part time. - We have 5 new starters who don't know what they are doing, but they make our sprint board look busy so management are happy Whilst I'm sure CI is great, and I agree with the disadvantages of PR's, there's advantages too, and they might be a good fit for your team. By all means taste the kool aid that the salesman is selling, but don't down it in one.
Having done CI and CD in previous settings (using trunk based development and pair programming), I can 100% agree with saying that PR's are both a blocking factor and not relevant to high performing teams. However, I now run a team with 100+ devs and have CI and CD running on trunk based development, and the entire team is remote with a high churn rate. Pair programming is nigh on impossible, especially considering there are only a very small pool of SME's which again diminishes the gains from the pairs unless they are working along side said SME's. What would be your solution to this conundrum? I'm sure there are many other companies out there that are also relying on a very high volume of remote workers giving the covid crisis we've experienced.
My solution would be a once a day code review meeting done on a single PR for the team. Ie, the team works on a sprint "trunk" during the sprint. Every developer pushes changes to it as they work. Once a day, the team gathers (perhaps in place of scrum standup) and goes over the committed changes since the day before, resolves issues and merges the PR. Obviously, your teams would have to be small (ie, no more than 5 devs).
In my experience programming works best when funny guys work together - physically at the same place, coding, stacking pizza boxes, joking, helping each other.
You are considering just one person doing Code Review. But if your board has a REVIEW column where all developers can review code you can speed up the reviews. And the developer who has permission to merge the Pull Request can just take a fast look into the code and merge it. It’s not a problem at all.
It doesn't remove the issue with offline reviews though, does it ? Review of code other than your own is usually not quite popular, as you have to break you own line of thought in order to figure out what another person wants to achieve. So you either don't get your code reviews done, or, if you force the team to reduce the review queue, reviews will most likely not beneficial to anyone and.code quality (your "quick look" approach 🙂 ). Which means you can basically skip it alltogether. The most beneficial way of doing reviews I found - besides pair programming - are online peer reviews, where the code creator explains his/her changes to an ideally competent other. Even if you do a review "rubber duck style", it provides a chance for self reflection on the coder's part.
Pair programming code reviews (online) keep the problem with multiple requests for a reviewer stop his work to do the review. Offline code review can be done by reviewer on free time, and github for example, has tools to do it. If you don’t use tools like github PR review, probably you have to do pair programming. If the reviewer reading the code doesn’t understand the written code, probably this reviewer is not a senior developer, so it’s being done wrong.
I don't believe in optimizing for a specific set of studies that seems to suggest a certain set of changes. We have no way of knowing if those studies are exhaustive, or affected by confounding factors (ie, top quality developers are always the ones who try the "new" thing).
Your team is probably not a bunch of Google coders with more than 10 years of experience. There is no 1 size fits all solution. Focus on the current pain points of your team rather than blindly applying to the extreme whichever new practice you hear about. Do stay informed on them and be critical on how they could benefit your team. Try them out if that's the case. On a side note the whole argument about pull requests not allowing you to have a 2 way communication is particularly bad but it serves the purpose of this click-bait video I suppose. Devs are usually smart enough to judge when a simple PR comment is enough or when it might be better to simply have a call to clarify certain things or maybe take the opportunity to teach a junior team member. Also, remember that shipping working code as fast as possible should not always be the goal. Working code is usually easy to achieve. Great code is working code another dev can read later on without wanting to murder you ;)
Perfectly said 🎉
13:20 glad you acknowledged the fact that Code Reviews are asynchronous. That’s the benefit (you can get to them when available) and people can work independently. Pair Programming requires synchronized schedules which winds up being harder to execute on when everyone has different things going on. In my own experience chasing bugs and working in a mature codebase, pair programming with an experienced IC helps a lot but those people are often not available. Pair Programming with someone just as (if not more) clueless actually slows you down. Have you paired with people with no experience?
That sounds plausible for the situation: a mature/legacy application in maintenance mode. But in another situation (ex. delivering a brand new product), asynchronisity can be a burden. It can result in a dev sitting idly by twiddling fingers or moving on to other work, which then causes juggling that can result in mistakes--the point he is making this entire vid. I do often pair with less/equally-experienced people. If you're both equally clueless, you don't have to share a screen; you both can explore and exchange ideas on the fly the come to a better understanding faster then you would separately.
"when everyone has different things going on" That is not a team.
@@dweblinveltz5035 I don’t disagree. Working asynchronously is not ideal from an Engineering standpoint but is often the reality when you need to deliver results for a business. Frankly speaking, working on a feature is asynchronous to development on the main or other branches. When you don’t even have a main branch yet, I agree, that pair programming likely more suitable.
@@andrealaforgia that has been the reality everywhere I’ve worked. Even, if you are working on the same codebase, it is on a different ticket than your colleagues. If everyone is working on the same ticket, that sounds like there isn’t enough work or the scope of the work is high and could be broken down and delegated further.
@@andrealaforgia Really? If you have a junior developer and a senior developer, it is expected for the senior developer to go in meetings and have higher lever discussions than what is expected from junior developers. Usually, a team has to divide their time into feature development and maintenance. And different people will handle different things. And this is, yet, a team. Now, if you have a team to handle a specific thing at a time and do only that, probably your team is really inefficient.
Great content, Dave! BTW, just waiting for my PR to be merged for last 3 days 😄
You have my sympathies, and thanks.
My dev shop is currently using short-ish lived branches with pull requests for a couple of reasons. (1) We are still catching up on our test automation and the coverage isn't there to bypass manual testing, meaning that the time from commit to deploy is longer than it should be. (2) Our SOC 2 process requires that we prove that code reviews happened. You alluded to one or two (ugly) ways you've done this in the past. What tactics have you seen for auditability of code reviews in a trunk-based development scenario? The pull request fills that need for us currently. Thanks for all the work you do on this channel. I consider Continuous Delivery to be one of the few books on my "career changer" list.
Thanks, pleased you liked my book. In the regulated environments I have worked in, we “tagged” commits, in the commit message, with the ids of the devs who paired on it. That was agreed to be fine with our regulators. This in finance (various), medical systems (various) and gambling (various).
I’ve supervised a larger “traditional health care company wants to go digital with little knowledge” project. We used github and I convinced the eng leads to contribute to main (to have them break stuff early) - BUT - a pre-commit hock would prevent a commit if not a github url to a github issue was found (aka the feature ticket). This had the nice side effect that other team members sometimes stumbled into other feature aspects and could attribute that commit to another feature easily instead of creating a new PR or merging stuff around. From an audit perspective: before the production push people would go through the recent active tickets, see the commits that are tagged into the issue (github is great for that) and simply put their “I have herewith reviewed the attributed code… bla bla” with a link to the release tag as a comment beneath that. That worked well and at an audit we were able to filter commits by issue / ticket id. Perfect? No. Better than having 20 PR branches? Oh yes. Hope that helps
I find this video spot on. In the first part of my career, we only practiced trunk based dev and pair programming way before these terms were well known. Compared with recent teams I worked in, with PR-hell and endless waiting for reviews, we produced higher quality code and had faster development. But most importantly, it was much more fun! Pair programming may look more difficult because in open source projects, people don't naturally work together, or at the same time. However, with some planning and use of new tools like Live Share in VSCode, it can easily be done. But the problem really is the same as for commercial projects - many find pair programming too intrusive, which is sad.
Yes! If you can make the work interesting or even fun, more thought will inevitably be put in, and human intelligence is one heck of resource to forgo whether deliberate or unintentional.
I agree that PR's are pretty inefficient; or fairly useless, if people just approve them without really reading them; which is what usually happens if the PR is more than a few lines of code and you mostly trust the one who wrote it. And I agree that "pair programming" or at least several developers working on the same feature more or less simultaneously is very useful. But literal pair programming, where theres two developers in the same room and only one computer is not sustainable at all. It is a great way to train junior developers; preferably having them type and the more experienced one mostly talking and monitoring... but it's exhausting! And unless you're very comfortable with the other one, it's also very uncomfortable feeling like you're continuously being evaluated. I'm an introvert (and in my experience, most programmers are) and I only have energy for about 2 hours a day of non-stop socialising. I work efficiently about 6-7 hours a day when coding alone; but after any social interaction like meetings* or pair programming for more than a few hours, I have no more energy and need at least a day to recharge. I much prefer to work physically alone most of the time, ideally rarely attending meetings. When my tasks are fairly trivial and low risk (like CSS styling and adding entirely new features that cannot break anything until the new component is actually added to a page), I'd rather just implement them on my own and push directly to master/main/root (or whatever the main branch is called); when working on less trivial and/or high risk features (like changing or adding to components that are already in use) I quite like working together with another developer, mostly asynchronously both working on the same branch regularly pulling the others work and regular informal discussion on text-chat or verbal in person or video chat. I actually prefer video chat from home over in-person chats in the office; mainly because of the stupid "open plan" offices used everywhere where it either feels rude to disturb everyone by talking because the room is full of quiet introverted developers; or where it's impossible to get any work done because the room also has a lot of loud extroverts constantly talking. (If we had individual rooms at the office with soundproofing, it would actually be better to have occasional in-person chats.) If this is what you'd call pair programming; I agree completely. (* at least the meetings where I need to pay attention and cannot, zone out working on code on my computer when the others talk. The "pointless" meetings where I don't need to pay attention is less exhausting; I can perhaps tolerate about 4 hours of that before I'm exhausted rather than only 2... Disturbingly about 80% of all the meetings I'm invited to are the pointless kind...)
"if the PR is more than a few lines of code and you mostly trust the one who wrote it" Seriously? Where I'm from, this doesn't happen very often.
@@names-mars what doesn't happen often? Trusting people or small pull requests?
@@names-mars Same. Our PRs are generally about five classes as a rule. They almost always generate useful discussion.
I could not agree more!!! My experience comes to the same conclusion. Pair programming is a great way to mentor too.
can we get the links to the studies you mentioned?
While I find the idea of finding a different approach to code reviews than using PRs is very intriguing (because they tend to prevent direct communication within a team), I don't think that pair-programming is the final answer to it. Pairing is great and I really like to do it, especially to onboard new people or train juniors, however, experience has shown me time and time again, that BOTH participants tend to get a common tunnel vision over the course of a pairing session, turning two pairs of eyes into a mutual single one. Therefore, the code quality is not necessarily better at the end and it is good to have a second look on the changes a bit later, with some distance. Additionally, pair programming becomes extremely straining when done remotely. Thus, I'm still on the lookout for some better techniques than PRs.
I agree. The amount of times I rejected a PR from a pair because they got tunnel vision and missed the point
I first tried pair programming, with a friend, back in the early 1990's (back in high school). We created a game together. Obviously we didn't call it pair programming way back then. We were forced into that situation, neither of us had laptops (way too expensive back then) and carrying a large PC + monitor to a friends place wasn't an option! We liked it and I can only recommend it - the code quality and focus was way better working together.
“With a friend…in high school” is the takeaway here. Now try it with strangers working on a CRUD app.
@@SM-ok3sz Oh please. That's not the takeway. The takeway is that working together, on the same machine, is possible. Colleagues are coworkers, not strangers.
@@andrealaforgia You’re right, strangers often smell much better than my coworkers.
What you're saying would work well for fully on-site teams consisting of very high quality developers. But this scenario is very rare.
For OSS and distributed teams, I think you can mitigate many of these issues while still using "develop" and "feature/*" branches. The following looks like git-lab flow but is effectively a delayed trunk-based workflow: * Rebase after every commit - Local post-commit hook rebases your feature branch with develop branch. - Server hook rejects pushes to feature branch if it is behind develop. - CI fails if feature branch is behind develop. CI also runs nightly * Anybody at any time can merge a feature branch into develop, w/o a PR - CI deploys develop to a testing server - CI creates a PR from develop to master. * Reviewed develop->master PRs reviews are only merged in historic order. with --ff-only - Merges are in order. A PR approval merges all past contiguous approved PRs, if any. - A merge to master causes a deployment to production - CI also runs nightly. If any PR is more than 24 hours old, merge it, but keep PR open. - PR rejections result in git revert on develop. This is effectively trunk-based development, but with a CD delay. (Assumes CI runs on for all commits for all branches.)
I'm generally against having separate dev and master branches, but --ff-only is the key difference here as it avoids creating unnecessary merge commits and simply treats the master branch as a pointer to the deployed commit.
Right. The 2 key features are feature branches are always up-to-date with develop, and develop is basically PR queue for CD.
PRs do mean that changes have sufficient comments in place to explain the code asynchronously. If you're having a conversation about it then you understand it, but all future developers may not.
Yes, but write USEFUL comments. //This is a stop sign StopSign(); Is a useless comment //This stops the sign from crashing into a wall as a result of a Microsoft bug see: (link) StopSign(); The first is just useless clutter. The second describes INTENT, which means the comment adds value.
@@Stettafire yep. Point being that a PR simulates how a future developer will read the code. Having a conversation means your code doesn't have to be explicable to a cold read.
When waiting for someone to do a code review for you, you can always go and review someone else's code, or start working on something else. It is not that you are a master and you are waiting for servants to do stuff around you. I have seen devs wasting much more than 15-20 mins playing FIFA. If code review requests come in every other moment and get you annoyed by, could be that either CI or teamwork does not work for you.
Yeah, you can use that waiting period to study something in programming, work on other tasks or just participate in reviews yourself. Not sure why waiting period is considered harmful because you can still do so much in that period. If you don't have tasks after you open a PR, then that's another issue entirely and not related to code reviews at all.
>or start working on something else. So assuming that your team structures work in small tasks (as advisable), what do you do? Start one task, wait for review, start another task, wait for review, start another one, and so on? You will end up building up work in progress, which is a very bad antipattern for teamwork. Have you ever heard "start less and finish more"? Code review requests coming in very other moment is not teamwork, it's annoying noise that impedes work. The solution is active collaboration.
Out of all of the developers I've worked with MOST do not like paired programming. I am 100% certain that if you do a survey of 100,000 developers you'd have at maximum10% (though in my experience it's been closer to 1%) that like paired programming all of the time, and another slightly larger chunk of people who like to do it SOME of the time. This isn't a hard problem to solve. You supposit only solution (even if you have to be creative you should always consider 3 or more). There's a LOT of solutions ranging from having a dedicated code reviewer, to having WIP limits that force people to clear the PR queue before they continue to a next change. The biggest problem with branch development that I've seen, is related to an unclear or unenforced standard around how big any single change should be, and what order of operations should be during the process. These are easily solvable through a little communication and experimentation. I appreciate the content of many of your videos, but this whole thing made me feel really upset.
The concept of pair programming and its benefits have been there for 20 years. Yet, no matter how much these evangelists scream about it, its practice is not widespread - rather than acknowledging its problems, they either tell you "you didn't do it right" or "you didn't try it long enough". They also blame management for not adopting it because they have the "mistaken" view it's an unproductive drain on resources rather than accept it's a brutal, divisive practice whose benefits don't justify 2 engineers worth 100K+ each, working on the same piece of trivial work.
Great video! How about code reviews after pushed to trunk/master? There are tools for that too.
Great video. I think you’re 100% correct… in a perfect world. The reality out there is way more complex, for different reasons. I’m someone that truly understands the benefits of trunk based, and the ONLY way of making it work is a full pair programming team, and I advise everyone to try it. But from my experience, most of developers feel uncomfortable (me included) and present higher levels of dissatisfaction and stress in the long run. So, is it faster? Yes. Is it sustainable? I have my questions.
This is a great take on this issue. I see the common retort whenever you criticize trunk based dev and pair programing, is to blame management clinging to wrong metrics or blame the programmer for not giving PP a fair go - but as you pointed out, the real world is more complex. Teams often have high turnover, devs who aren't passionate, devs who are highly introverted, shortage in hiring devs, devs working remotely in different timezones... the list goes on. TBD and PP are effective ways of working, but isn't the silver bullet for every team.
What about the risk of an echo chamber meaning a review is not necessarily as extensive as new eyes
16:20 I am a huge believer in pair programming because of my experience rewriting, from scratch, the entire Florida Real-time Vehicle Information System (FRVIS) in the late 90's. I noticed that when two people were producing the code, one person took the executive function (the checklist of things that needed to be addressed) while the other took the manual function of actually typing in the code. The "navigator" also could inform the "pilot" of typos in the code, or other syntactical errors. The code would go out with near zero flaws EVERY time! When I compared this to my experience of our team producing changes via single programmer and all of the hours tracking down bugs or hopping on a problem only to determine that it was exactly what the user had requested, I thought "Screw the managerial idea that maximizing productivity is having each person work on a task, the answer is to complete the task right the first time and two people towards that goal is NOT wasting of resources!" I'm glad to find someone who validates my observation :-)
Also, when the pilot got tired, we could switch places and keep going, thus supporting the next point that we could code longer at a sustained speed with the same low error rate.
I support this with my experience, but I've got only single dev I paired with - at the time there were only two of us and we wanted to try it out. I also tried it later with someone else and I figured that successful pairing definitely depends on a personality you are pairing with. With some people it's so hard, that I'm better be alone.
I love PR:s for history's sake. I'll write descriptions with screenshots, and potential alternative approaches I considered using and why I didn't. I often have multiple commits and squashmerge the PR for a tidy master history - but the ability to go back and view the details of the PR if needed. Automation is of course also set up so a server runs my full test suite on a PR during my break and I don't have to run them on my local machine.
"...and I don't have to run them on my local machine" is a red flag. Let me explain. If it is difficult to run the build and tests locally, if it is slow, if you have to perform 17 manual steps, if it downloads the internet - that is feedback. That is THE feedback you should LISTEN to. One of the major points of a local build is to act as an artificial constraint to expose technological, design and workflow issues. Replacing the local build with a PR build on a build management system is a band-aid so that people can avoid fixing the real problem.
I totally agree with everything here and I have worked using these practices to much success. However I have a use-case for pull requests. Open source software benefits from pull requests, where the software is maintained by a community and communication is inherently asynchronous
Any suggestions on dealing with less trustworthy teammates who also don't love pair programming? We had a bunch of situations where devs were creating PRs with untested changes (or without tests) that would create a real mess in production, and the same people think that tests are the necessary evil instead of something that should be, if not the first step, then an essential step in programming. I would really love to ditch PRs, but I don't think it would be a good idea because of things like this.
Make unit test part of your definition of done.
Can’t say I agree with the ideas here, it’s hard to put my thoughts down in a comment, but I work in a field where there are serious consequences if software isn’t tested fully so releasing new builds daily is not feasible for my team
1. Never was it said you have to release you software to production every time you push to trunk. It can be done in a SAAS situation, but in other cases it indeed may not be practical (embedded, ...) 2. Automate the tests. If your full test suite runs in more than 8 hours, you should optimize it.
Building off Laurent's response, you can easily setup a CI/CD pipeline with something like GitHub actions and perform unit tests every single time code is pushed. This doesn't require you to release a new build, rather, it ensures that each step is properly executed as it is completed rather than doing a massive review at the end of that development cycle
I was once assigned to a team (to eventually take over it), and PRs were just a formality. We had a suspicion that was the case, and I proved it by making a PR with a comment line in the code saying "the line below this comment breaks the code, please comment on the PR and do not approve". The PR was approved.... Needless to say that the quality of the code that the team has been working on was horrible. Each team member also had an "area of expertise", so knowledge was silo-ed.. lots of context was lost the moment someone leaves. Pair programming helped solve a lot of this problems.
Interesting thoughts; My main questions that remains is how to best implement pair programming in an asynchronous remote setting though, especially in the early days of a small team when there often may just be one person working at any one time, with only 2-3h of overlap - right now I prefer automated testing to commit to main branch and then automated end-to-end testing to mark as release ready and then one manual review a day to approve the changes, but it also feels like the review is happening too late for good feedback loops, but at the same time it doesn't slow us down, ensures at least one release a day and doesn't force people to pause life to be somewhere with someone else at the same time.
My advice is pair as much as you can. If you have a small timezone overlap, encourage remote pairing during that overlap. It strengthens the relationships and builds trust between team members better than anything else that I know. So it is even more useful when working remotely like this. I spent a couple of years working most closely with a team based in Chicago while I was based in London, we usually had 1 or 2 hours of overlap, but we paired when we could and the team was strong, I think, in part because of that.
Very interesting video, thank you. However I quite disagree with the ideas presented. Firstly pair programming is a really helpful way to work, however it comes with a huge issues - most problematic is to find common time (I have a lot of meetings during the day, developers in my team have less meetings, however there are some. Also I'm constantly being asked for an advice or help from other devs/analysts/testers/managers.... from other teams - this would mean that my "partner" must wait until I finish with the call or planned meeting). Also there is a small language barrier. The final effectivity is definitely much less than 2 devs working separately, at least from my experience. There are definitely a good uses of pair programming, e.g. for some more complex design, for trainings etc. Secondly, pullrequest reviews are not one-way communication - if there is something unclear, we can meet or call to resolve the issues and explain reason why the code is written in some way or why it'd be better to write it differently (perf issues, security issues, readibility, ... ) Lastly you can always have a PR policy to "test merge" the changes from main branch and to run CI pipeline., so there is not that much time spent waiting for a feedback. I believe that you're missing one point and that's testing. It's fine for smaller teams, but in larger products (tens of devs in single codebase) there are constantly some new bugs created that would make problems for other devs to work on their own features. And ofc, all devs test their changes and run tests, but the application can run within a lot of totally different setups, therefore the problem might appear only for someone.
What could be the reason, to wait for an approve, before "submitting" to CI? Why not, test the pull request, merged with the changes?
I honestly thought this was where he was going to go and was surprised when 'pair programming' came out of leftfield. I'm in the middle of architecting the validation flow of a project where I'm trying to introduce TDD in an organisation that doesn't really do it, and where I certainly don't have authority to implement pair programming. The first thing I realised is that any coding standards that can be automatically checked should have those checks performed and recorded *before* anyone is expected to review the code. Have the CI system prove that it builds and passes static analysis. Once you're there, though, why not make passing of some of the test suite (or even *all* if it's quick enough) a prerequisite as well? Then the code review becomes less "will this work?", because you've already proven that it does, and more "is this maintainable?" Then you can have a larger, once-a-day review to roll up the already tested changes, rather than lots of small interruptions.
[13:56] "I prefer a conventional code review where we have a two-way conversation" (over async pull requests).
i understand the point which pull request is not the best option to delivery fast, but, in my experience, pair programming was not that good, sometime was most about one developer coding and other watching. In the end it was like a sequence of everything that was talk in the daily meeting. The best code review practice that I saw in my career: one developer coding, when finish he call the other and explain the code, in the process the same developer who is explain see the wrong points on his code
I hate pull-requests for precisely all of the reasons you've mentioned. However, I've also been in a situation in which management suddenly decided to take on-board a large number of inexperienced developers who would produce overwhelming volumes of junior-grade code, riddled with security and architectural holes. I don't think that pull-requests were the answer to that situation as they lead to burnout and resentment amongst developers, but I don't think your suggestions could have saved that situation either. As with most things complicated, to have success you have to have maturity. The systems you're proposing are for companies run by mature people who can afford to attract mature employees, who can set-up and maintain mature development pipelines. Whilst I don't argue with the statistics you quote, I do wonder whether they're reflecting the success of maturity more than good development practices. I'm not saying you're wrong. I like a lot of your ideas on this channel. Just that your advice may be aimed at people who either don't need it, or won't listen to it.
my org use PR as convenient revert button lolz
"Hire better people" always seems to be the solution to all objections to continuous integration. Who, then, hires juniors and trains the next generation?
@@EricMuranoAU You're right, but I didn't mean my comment to be taken that way. It's important to hire and train junior developers, but not so many that more experienced developers just become babysitters. Matching the complexity of the problem you want to solve to the complexity that a team can handle sustainably is one of the most important to get right.
Sorry, I meant that as a response to the video, not to you. In my mind you and I are on the same page
@@EricMuranoAU No hard feelings ;-)
I think the review part is actually the smallest problem. I agree that it's better done talking to the other developer rather than writing back and forth in comments. But what about all the other quality gates? Static analysis must pass. Code coverage must pass. Tests must pass. Etc. How do you avoid getting code on trunk that fails automatic quality gates?
Copy paste of my other answer: You do! You run the checks. Before you push. You make sure to merge other peoples changes from remote/main to your local clone many times per day, and run the build, unit-tests, linting, and functional tests **before** you push to master. That way you know if there is a conflict before you push. This is the prime directive of CI - check your sh*t before you push. And if it still breaks on the build servers, stop working, drop everything and fix it, or revert the breaking changes within 10-15 minutes to unblock the rest of the team. If you can't do these things on *any* environment (including your local env) you need to fix that.
@@ddanielsandberg And if I make all the tools available locally - which would be expensive - and one of the checks fails an hour before I'm off to vacation, and I realize this will take 10 hours to fix, and someone should take over. What do I do? Can I create and push a branch in that case?
@@adagio1980 If a simple revert doesn't fix it and it takes 10 hours to fix you probably stumbled onto a nasty systemic bug that no one knew about. Concurrency, transactions, etc... Since we in CI talks about "push at-least once per day, preferrable multiple times per day". The amount of "work lost" if that is left on you laptop while you're on vacation is neglible. The amount of times I've started working on something on Friday afternoon, got stuck, went home, came back after the weekend and just deleted whatever I was doing last week and then implemented a much better solution in a few hours...
What I describe in this video is part of a bigger approach. I call it Continuous Delivery, but in that approach we are disciplined, but also flexible. The goal is to optimise for fast, clear, definitive feedback. For lots of languages, you can get answers to some of those quality gates in you IDE, so do that. For others, add the checks to the commit stage of your deployment pipeline. Whatever the gate, you optimise to get the answer as early as you can. The Deployment pipeline should be definitive for release, and should be able to give you an answer multiple times per day, so you optimise whatever you need to to make that possible. There are examples of orgs and teams doing this, sometimes at massive scale with very complex stuff, so it works, you just have to make it work.
This video accurately describes the scenario. Pull Request is mere a ceremony and the code is approved without much deeper inspection. Pair programming is the only solution as you mentioned. Pair programming shares the know-how, allows easier integration into the team, and increases trust among the workers. Still, some team promotes Pull requests... Yes, it is for a long-running slow Software system where someone wants to contribute like open source. But it is certainly not for CI/CD.
At my current client (I'm a consultant), the only thing that's holding me back is their extremely over-engineered and convoluted CI/CD-pipeline, including but not limited to the pull request management. When fixing a bug, the bug itself usually only takes minutes to identify and fix, wheras actually prepping the fix for deployment and performing the actual deploy takes several hours. It's a serious cramping of my style compared to the extremely effective trunk-based development I've done at other clients. The only "good" thing about this is that I get paid for several hours of work for bug fixes that only take minutes to fix.
Pair programming is not a substitute for PR's. PR work great, usually the problem is the size of it.