Why Pull Requests Are A BAD IDEA

Поделиться
HTML-код
  • Опубликовано: 21 дек 2024

Комментарии • 1 тыс.

  • @benbaert2166
    @benbaert2166 2 года назад +576

    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.

    • @joonasmakinen4807
      @joonasmakinen4807 2 года назад +14

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

    • @lantero88
      @lantero88 2 года назад +17

      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.

    • @squ94wk
      @squ94wk 2 года назад +35

      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.

    • @anacierdem
      @anacierdem 2 года назад +4

      ☝️This

    • @AdamJorgensen
      @AdamJorgensen 2 года назад +3

      @@squ94wk Agreed. It's not for me

  • @sciros
    @sciros 2 года назад +80

    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.

    • @drogin88
      @drogin88 2 года назад +1

      Exactly my thoughts

    • @ContinuousDelivery
      @ContinuousDelivery  Год назад +7

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

    • @einfacherkerl3279
      @einfacherkerl3279 Год назад +2

      @@ContinuousDelivery hiring should be based on buy one get 1 free perhaps?

  • @elguaro76
    @elguaro76 2 года назад +81

    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.

    • @wojtek1582
      @wojtek1582 2 года назад +3

      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 .

    • @maimee1
      @maimee1 2 года назад +1

      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.

    • @ddanielsandberg
      @ddanielsandberg Год назад +1

      Run the full build and tests locally before every push! No need for branches.

    • @JayCawb
      @JayCawb Год назад +2

      That's just not practical for everyone. Our "full build" is over 200 jobs and takes hours to finish...

  • @sto3359
    @sto3359 2 года назад +388

    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.

    • @_winston_smith_
      @_winston_smith_ 2 года назад +43

      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.

    • @CordieBram
      @CordieBram 2 года назад +22

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

    • @Quenjii
      @Quenjii 2 года назад +43

      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.

    • @barberaf
      @barberaf 2 года назад +25

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

    • @_winston_smith_
      @_winston_smith_ 2 года назад +7

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

  • @moodynoob
    @moodynoob 2 года назад +298

    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 😂

    • @angepicard7968
      @angepicard7968 2 года назад +32

      Blindly applying any practice is a recipe for disaster! :O

    • @joshmccall
      @joshmccall 2 года назад +37

      #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

    • @inasuma8180
      @inasuma8180 2 года назад +14

      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.

    • @Syntax753
      @Syntax753 2 года назад +11

      Certainly pair programming should not be 8 hours a day! That will be give me nightmares

    • @asicdathens
      @asicdathens 2 года назад +5

      I worked in a place where one of the teams went on mob programming. Not the best outcome

  • @MurariAlex
    @MurariAlex Год назад +8

    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

  • @MichaelPetito
    @MichaelPetito 2 года назад +90

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

    • @Stettafire
      @Stettafire Год назад +9

      100% this. Have small PRs and short lived branches that are easy to review and have good test coverage.

    • @bishop3000home
      @bishop3000home Год назад

      100%

  • @thehuggz-i9k
    @thehuggz-i9k 2 года назад +63

    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.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +11

      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.

    • @brokenphysics6146
      @brokenphysics6146 2 года назад +11

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

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +4

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

    • @andrealaforgia7699
      @andrealaforgia7699 2 года назад

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

    • @brokenphysics6146
      @brokenphysics6146 2 года назад +8

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

  • @sarqf212
    @sarqf212 2 года назад +89

    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

    • @k-yo
      @k-yo 2 года назад +11

      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.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +11

      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.

    • @sarqf212
      @sarqf212 2 года назад +15

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

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +2

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

    • @sarqf212
      @sarqf212 2 года назад +8

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

  • @FLAMESpl
    @FLAMESpl 2 года назад +13

    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.

  • @sauravprakash2743
    @sauravprakash2743 2 года назад +56

    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.

    • @carlosmspk
      @carlosmspk 2 года назад +3

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

    • @thekwoka4707
      @thekwoka4707 2 года назад +1

      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.

    • @dieSpinnt
      @dieSpinnt 2 года назад +2

      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:)

    • @13b78rug5h
      @13b78rug5h 2 года назад

      The first question is why you have a team with different timezones? Async collaboration will always have disadvantages over mostly sync collaboration

    • @silentwater79
      @silentwater79 2 года назад +1

      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.

  • @sp-niemand
    @sp-niemand 2 года назад +68

    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.

    • @angepicard7968
      @angepicard7968 2 года назад +1

      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.

    • @chaos_monster
      @chaos_monster 2 года назад +6

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

    • @andrealaforgia
      @andrealaforgia 2 года назад +1

      Once again, pair programming does not mean "working while there is a person watching every LoC over your shoulder". That is NOT pairing.

    • @prozacchiwawa
      @prozacchiwawa 2 года назад

      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.

    • @sp-niemand
      @sp-niemand 2 года назад

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

  • @archibaldbuttle7
    @archibaldbuttle7 2 года назад +13

    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.

  • @ArchStalker
    @ArchStalker 2 года назад +21

    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.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +6

      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.

    • @ArchStalker
      @ArchStalker 2 года назад +1

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

    • @13b78rug5h
      @13b78rug5h 2 года назад +3

      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

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад

      @@13b78rug5h I have seen that too for some people. Thanks for sharing your experience.

  • @TheTwingg
    @TheTwingg 2 года назад +3

    What could be the reason, to wait for an approve, before "submitting" to CI?
    Why not, test the pull request, merged with the changes?

    • @FraggleH
      @FraggleH 2 года назад

      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.

  • @BradleyArsenault
    @BradleyArsenault 2 года назад +19

    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

    • @vyli1
      @vyli1 2 года назад

      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.

  • @felipelopes3171
    @felipelopes3171 2 года назад +28

    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.

    • @EngineerNick
      @EngineerNick 2 года назад +9

      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.

    • @thekwoka4707
      @thekwoka4707 2 года назад +3

      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.

    • @andrealaforgia7699
      @andrealaforgia7699 2 года назад +1

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

    • @andrealaforgia7699
      @andrealaforgia7699 2 года назад

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

    • @felipelopes3171
      @felipelopes3171 2 года назад

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

  • @Volte6
    @Volte6 2 года назад +15

    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.

    • @Stettafire
      @Stettafire Год назад +1

      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

  • @philipwilkinson7724
    @philipwilkinson7724 2 года назад +3

    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.

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +1

      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"!?

    • @etherweb6796
      @etherweb6796 Год назад

      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.

  • @KogiSyl
    @KogiSyl 2 года назад +9

    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 ;)

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +2

      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.

    • @KogiSyl
      @KogiSyl 2 года назад

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

    • @moodynoob
      @moodynoob 2 года назад +1

      Tesla is a bad example as they are notorious for poor build quality - they're ranked 1 before last by Consumer Reports for reliability.

    • @kasperstergaard1592
      @kasperstergaard1592 2 года назад

      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.

    • @DanielJoyce
      @DanielJoyce 2 года назад

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

  • @myronww
    @myronww 2 года назад +14

    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

    • @reav3rtm
      @reav3rtm 2 года назад +6

      Yes. This guy seems to sell the idea that CI/CD and trunk based development as the only workflow where regression tests are executed.

    • @LutgerBlijdestijn
      @LutgerBlijdestijn 2 года назад +7

      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.

    • @reav3rtm
      @reav3rtm 2 года назад +4

      @@LutgerBlijdestijn That would undermine his CI/CD narrative and selling point for his books. Why I cannot treat this guy seriously anymore.

  • @prdoyle
    @prdoyle 2 года назад +1

    Our pre-merge automation does a "trial merge" with the main branch and runs the test suite against that. You can get that "green light" without actually merging to the main branch; and you can re-push your changes every few minutes if you like, without bothering anyone. When the "unit of work" is done, you ask for a review and typically merge later that same day. What do you think of that practice?

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +1

      This has been brought up many times on his previous videos about branching and git flow. The answer is still the same:
      1. All the different feature branches will **not** be integrated with each other since they are isolated until they are on mainline. Until it's on main, it's not integrated.
      2. The unit of work in FB/PR is quite large and is regarded as a "complete whole". In CI/CD/TBD we see the unit of work as a (couple of) small commits, many times per day, so that everything is always integrated and tested, even if incomplete.
      3. If you really must work that way, fine. Still not optimal and it's not CI whatever anyone calls it. There are better ways...
      Conflicting perspectives:
      - FB/PR proponents views this isolation of changes (and programmers working alone) as a good thing.
      - CI/XP proponents views this isolation as undesirable and a fault in the system of work.

    • @prdoyle
      @prdoyle 2 года назад

      @@ddanielsandberg thanks for the answer. Food for thought, for sure. I'd argue though that using a "trial merge" with the main branch is indeed integrating your change with everything else that's merged. It's not integrating with half-baked work that still has its own bugs and hasn't had a code review; I'm not really seeing why one would want this?

    • @ddanielsandberg
      @ddanielsandberg 2 года назад

      ​@@prdoyle Wall of text incoming - duck and cover!
      1. Sure, but it is not integrated until it's ON main.
      2. Even if you rebase your branch, you still only check integration with what has made it into main this far. Everything else that is not on main is an unknown.
      As an example: If everyone "merge into main" at the end of the day or end of sprint, you won't know if everything actually integrates until that time. That is way to intermittent and is similar to old school waterfall "integrate and test at the end", although at least it's a more frequent than once per year. If everyone merges their code into main several times per day - fine - but what do you need FB/PR for in the first place if it only lasts a couple of hours anyway? Seems like a lot of ceremony.
      I have actually been there, twice, kind of - in a highly compliant industry. We had armies of testers, reviewers, configuration management people, merge bosses, process and branches, and yet we had very expensive production issues all the time. Then I took over as tech lead for the most certification-critical components. The only changes we made was that we skipped all the ceremony/merges/handoffs and started doing proper XP/CI/TBD and our response time became 10x better and our production issues went down by 2 orders of magnitude. That alone saved the company $2M per year. A few years later a bunch of managers decided that "everyone must do FB/PR because we have to be compliant (BS, yadda, blah) and we read on the internet that that is everyone else is doing so we should too" and it became dogma and enforced by people not doing the actual work. After that we had slowed down and were right back in the tank and half of us had left within a year.
      **We didn't need FB/PR - we were doing better without them already!**
      Because so many are used to treat version control as a kind of personal junk-drawer where we throw things into a branch, fiddling around for a while, have it being broken for hours/days, flaky and unstable and then pick up the pieces into a whole at the end, then squash merge and PR; they get stuck in that cycle.
      1. This is often a sign of bad design or issues with the culture.
      2. There is a world of difference between "half baked", "incomplete" and "messy crap" - in XP/CI the software should still always work properly.
      3. Code reviews are primarily there for learning and feedback. Async blocking reviews actually slows the organization down and have questionable effect on the quality. And even so not all changes requires review and not all reviews have to be blocking - see Martin Fowlers 3 types of reviews "ship-show-ask". FB/PR proponents are stuck 100% on "ask". You can always refactor and improve things after the fact, after it's on main, and even after it's gone live. Something people seem terrified of.

  • @hhappy
    @hhappy 2 года назад +39

    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.

    • @bobthemagicmoose
      @bobthemagicmoose 2 года назад +3

      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.

    • @bobthemagicmoose
      @bobthemagicmoose 2 года назад

      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?

    • @Macknull
      @Macknull 2 года назад +1

      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.

    • @joinnyful
      @joinnyful 2 года назад +2

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

    • @Sergio_Loureiro
      @Sergio_Loureiro 2 года назад +2

      Dave has already shared some of his thoughts on low-code platforms here ruclips.net/video/uxBZFju0Mjs/видео.html

  • @WilsonMar1
    @WilsonMar1 Год назад +1

    [13:56] "I prefer a conventional code review where we have a two-way conversation" (over async pull requests).

  • @sebastiannyberg3008
    @sebastiannyberg3008 2 года назад +14

    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.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +4

      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.

    • @andrealaforgia7699
      @andrealaforgia7699 2 года назад

      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.

  • @shhac
    @shhac 2 года назад +2

    Why does waiting for a code review on a pull request mean you're blocked? If you have a further change/feature depending on your first PR you can branch off the submitted branch and continue working.
    What part of the CI pipeline is 100% required and unable to do locally, e.g. connecting to a local or staging environment, and if you can't do it locally how can you write trusted code?
    The chance that a reviewer would completely veto your signatures or interfaces to the point it isn't a simple change to match is small, and if it does happen in a way which breaks the latter feature then there were poorly defined scope boundaries between the two pieces of work.
    Later, as the stacks pass review you can simply merge things in as usual.
    Of course a code review which raises loads of issues and you can't learn from is bad, but this is an issue with the style of review - not the process.
    Similarly, if you can't rebase code easily then this suggests the individual commits contain too many changes and should be smaller, or you've gone in different directions and should tidy up history by squashing certain commits together as fixups before submitting the branch.

  • @johnnyblue4799
    @johnnyblue4799 2 года назад +7

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

    • @НиколайТарбаев-к1к
      @НиколайТарбаев-к1к 2 года назад

      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.

  • @sauliustb
    @sauliustb 2 года назад +1

    how do you do large sweeping changes in a CI setting then? for example: change the designated java version, switching from some library to another, ...

  • @babgab
    @babgab 2 года назад +32

    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.

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +2

      "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?

    • @babgab
      @babgab 2 года назад

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

    • @Ryosuke19
      @Ryosuke19 2 года назад +1

      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.

    • @babgab
      @babgab 2 года назад

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

    • @Ryosuke19
      @Ryosuke19 2 года назад

      @@babgab Well i certainly would not call it pair programming if only one person is actually programming.

  • @AlexAegisOfficial
    @AlexAegisOfficial 2 года назад +11

    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.

    • @youngwt1
      @youngwt1 2 года назад +3

      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

    • @andrealaforgia
      @andrealaforgia 2 года назад

      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.

  • @ubiratansoares
    @ubiratansoares 2 года назад +34

    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 🙂

    • @derVilli
      @derVilli 2 года назад +5

      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.

    • @ubiratansoares
      @ubiratansoares 2 года назад +11

      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

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +13

      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.

    • @derVilli
      @derVilli 2 года назад +1

      @@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"

    • @andrealaforgia
      @andrealaforgia 2 года назад +1

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

  •  2 года назад +2

    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.

  • @grantwarrennics
    @grantwarrennics 2 года назад +6

    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.

    • @michaelrstover
      @michaelrstover 2 года назад

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

  • @sitedel
    @sitedel 2 года назад +1

    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.

  • @PelFox
    @PelFox 2 года назад +14

    One question though. Doing trunkbased, when do all tests run? Because the code is already merged to master before doing test runs. Which means another dev might pull bad code. Doing pull requests isolates test runs for the PR before it's being merged.

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +3

      You do! You run the tests.
      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 you need to fix that.

    • @topperthehorse
      @topperthehorse 2 года назад +5

      I have starting using PRs with automatic merging, which I think provides the best of both worlds, IMHO.

    • @amitev
      @amitev 2 года назад

      @@topperthehorse when do they get merged automatically?

    • @topperthehorse
      @topperthehorse 2 года назад +2

      @@amitev when all the CI tests have passed

    • @johnsmodularjams2123
      @johnsmodularjams2123 2 года назад +1

      @@topperthehorse at what point is the code reviewed?

  • @Faboslav
    @Faboslav 2 года назад +1

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

  •  2 года назад +3

    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.

  • @shavais33
    @shavais33 2 года назад +3

    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.

  • @mecanuktutorials6476
    @mecanuktutorials6476 2 года назад +29

    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?

    • @dweblinveltz5035
      @dweblinveltz5035 2 года назад +3

      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.

    • @andrealaforgia
      @andrealaforgia 2 года назад

      "when everyone has different things going on"
      That is not a team.

    • @mecanuktutorials6476
      @mecanuktutorials6476 2 года назад +1

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

    • @mecanuktutorials6476
      @mecanuktutorials6476 2 года назад +3

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

    • @PauloDanielCarneiro
      @PauloDanielCarneiro 2 года назад

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

  • @pilotboba
    @pilotboba 2 года назад

    What does your pipeline do in that first 5 minutes?
    Compile I assume.
    What else?
    Code Formatting Validation?
    Code Analysis (FxCop style)?
    Run Unit Tests?

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад

      I usually include: compilation, unit tests, static analysis, coding standards, data-migration unit-tests (if any), early indication of “breaking changes” e.g. changes in public APIs, and creation of deployable release-candidates on success.

  • @djhoese
    @djhoese 2 года назад +12

    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.

    • @hanzladev
      @hanzladev 2 года назад +1

      Agee, had same thoughts and pushed that to in comment section 😜

    • @BementalSea
      @BementalSea Год назад

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

    • @djhoese
      @djhoese Год назад

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

    • @BementalSea
      @BementalSea Год назад

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

    • @djhoese
      @djhoese Год назад

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

  • @johanullen
    @johanullen 2 года назад +2

    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.

  • @neildutoit5177
    @neildutoit5177 2 года назад +35

    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.

  • @murilosilvabr
    @murilosilvabr 2 года назад +2

    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.

    • @stephanklein257
      @stephanklein257 2 года назад

      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.

    • @murilosilvabr
      @murilosilvabr 2 года назад

      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.

  • @OLApplin
    @OLApplin 2 года назад +3

    In what kind system are you commiting to master every 15 minutes??? Do you commit changes without validating it yourself first??? Please help me understand, I only have a few years of experience in the industry and that makes absolutely no sense to me.

    • @gamer-gw9iy
      @gamer-gw9iy 2 года назад +1

      No, you obviously run tests locally before you push, you don't want to break anything - You push to master often to check if anyone else's changes break yours (because yours worked locally), and if so adjust either yours or their changes accordingly. It is common in TDD to use red green refactor, you'd only ever want to push changes if you're green 👍

  • @dotJEM
    @dotJEM 2 года назад +2

    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.

  • @roberthogan6901
    @roberthogan6901 Год назад +3

    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.

    • @Stettafire
      @Stettafire Год назад +1

      I agree. The amount of times I rejected a PR from a pair because they got tunnel vision and missed the point

  • @JayVal90
    @JayVal90 2 года назад +1

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

  • @dtasevski
    @dtasevski 2 года назад +3

    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.

    • @touristtam
      @touristtam 2 года назад +2

      Make unit test part of your definition of done.

  • @craigiedema1707
    @craigiedema1707 2 года назад

    Dave - with changes to work practices (WFH etc.) how do you practically implement pair programing?

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад

      Pairing works fine remotely. All you need is a shared repo, and the ability to share a screen and talk to each other. I spent about 18months being part of a team based in Chicago, while I was based in London, we paired for all the time that our timezones overlapped.

  • @BBdaCosta
    @BBdaCosta 2 года назад +70

    I love when Dave confronts the status quo and brings new ideas that are not "new" but very consolidated.

    • @pgeorgiadis2
      @pgeorgiadis2 2 года назад +7

      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.

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +5

      ​@@pgeorgiadis2 Wait, what makes you think that pair programming costs 100% more?

    • @andrealaforgia7699
      @andrealaforgia7699 2 года назад +1

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

    • @therealdecross
      @therealdecross 2 года назад

      ​@@pgeorgiadis2 Have you tried it? What can you share about the experience?

  • @joonasmakinen4807
    @joonasmakinen4807 2 года назад +2

    So, are you suggesting to replace pull requests with pair programming?

    • @joonasmakinen4807
      @joonasmakinen4807 2 года назад

      I see now it could enable continuous review, if utilized properly: ruclips.net/video/aItVJprLYkg/видео.html

  • @aralyon
    @aralyon 2 года назад +16

    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.

  • @davebywaters8187
    @davebywaters8187 2 года назад +1

    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.

  • @youngwt1
    @youngwt1 2 года назад +6

    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

    • @zerettino
      @zerettino 2 года назад

      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.

    • @2010karatekid
      @2010karatekid 2 года назад

      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

  • @Titousensei
    @Titousensei 2 года назад +1

    Can't you do both in parallel? CI can start as soon as the PR is submitted, and the reviewers can look at the PR when CI is green, or as soon as they want.

  • @dominiqueruest4780
    @dominiqueruest4780 2 года назад +3

    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 ;)

    • @moodynoob
      @moodynoob 2 года назад

      Perfectly said 🎉

  • @attila2246
    @attila2246 2 года назад +1

    I don't think Pull Requests are the problem, it's just about finding what works for your team. On my current one we PR every 2-3 hours and they're usually reviewed within 20 minutes.
    In the meantime I work on another branch and then rebase master into the branch after my previous PR has been accepted.
    Perhaps it's just inexperience but I've yet to see any downsides to this approach.

    • @moodynoob
      @moodynoob 2 года назад

      TBD and PP are tools and strategies in a toolbox. You should be aware of their pros and cons and use what's appropriate for your team. Experiment as well. I've experienced the problems mentioned by Dave, so these issues definitely can exist with teams using PRs - but in my case, these were due to cultural/management issues as opposed to the PRs themselves (in my opinion).
      However I've also worked with other expensive consultants like Dave, who espouse this stuff like cult leaders (TBD and PP are NOT new or novel practices) - and they're happy to take credit when their ideas work but cut losses and blame the client's management and culture when they don't work.
      By all means continue learning from what Dave has to teach, but don't fall into the trap that it's the only correct way to approach software dev and every other variation is bad.

  • @dungimon1912
    @dungimon1912 2 года назад +8

    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.

  • @alvidr
    @alvidr Год назад

    I really like pairing and understand why it's better than PR reviews and how pairing can minimize time or completely eliminate PR reviews, but what I don't understand is the techic described by the author when you commit changes directly to trunk first and request PR review on code quality later on. How does this supposed to work without feature branches?

  • @adagio1980
    @adagio1980 2 года назад +4

    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?

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +1

      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.

    • @adagio1980
      @adagio1980 2 года назад

      @@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?

    • @ddanielsandberg
      @ddanielsandberg 2 года назад +2

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

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +1

      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.

  • @suiko619
    @suiko619 2 года назад +1

    What about remote teams in different time zones who can't pair-program all day? How can they achieve continuous delivery?

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +1

      Pair where timezones overlap, pair with locals where you can, and fall back on something less good if you are the only person in a non-overlapping timezone.

    • @retagainez
      @retagainez 2 года назад

      I have this question too. But I think the answer is maybe that it requires a little bit of sacrifice. For me, I would have to pair in the early mornings because of an 8 hour time difference. I'm willing to wake up a couple of hours earlier just to be able to bond with teammates and get some work done :)

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +1

      @@retagainez Yes, I used to work with a team in a different timezone, so I shifted my work pattern a bit so our overlap worked out a little better.

  • @StephenMoreira
    @StephenMoreira 2 года назад +4

    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.

    • @ac.f.525
      @ac.f.525 2 года назад

      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?”

    • @StephenMoreira
      @StephenMoreira 2 года назад +1

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

    • @kasperstergaard1592
      @kasperstergaard1592 2 года назад +1

      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.

    • @StephenMoreira
      @StephenMoreira 2 года назад

      ​@@kasperstergaard1592 This sounds like an interesting approach, too bad about the discontinue. Thanks for sharing.

  • @williamrichards5241
    @williamrichards5241 2 года назад

    Seeking comment: could perhaps a commit (or PR) be used to tag the: red, green, refactor? Commit or PR is rejected if the integration doesn't pass. Including if no green, red, or refactor steps are found? Perhaps open some exclusion tags for edge cases where it doesn't make sense to have normal TDD but other tests/validation. The remote commit or PR could include evidence of prior red, and green, if no prior coverage exists?

  • @helidrones
    @helidrones 2 года назад +4

    In my experience programming works best when funny guys work together - physically at the same place, coding, stacking pizza boxes, joking, helping each other.

  • @curiouslycory
    @curiouslycory 2 года назад +1

    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.

    • @moodynoob
      @moodynoob 2 года назад

      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.

  • @patches152
    @patches152 2 года назад +3

    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

  • @PaulSebastianM
    @PaulSebastianM 2 года назад

    What do you tell organizations that want every commit to the main branch to equate to only a releasable feature or user story? For example, if multiple people are committing to main, each person should commit only once for when they are done with their US, it's like saying "this is the commit for the user story I was working on in this sprint, it's done, tested, accepted, ready to deploy".

    • @zerettino
      @zerettino 2 года назад +1

      I can understand the appeal of that requirement : it creates a very good looking commit history. But is that the goal you're after ? Does it add value or diminish costs ?

    • @PaulSebastianM
      @PaulSebastianM 2 года назад

      @@zerettino thanks.

  • @Bennevisie
    @Bennevisie Год назад

    @Continuous Delivery How would pair programming work in the event of a bug or issue that needs troubleshooting? Especially where research needs to be done in order to eliminate or hone in on different possibilities? Is it not perhaps more productive to do work individually?

    • @ContinuousDelivery
      @ContinuousDelivery  Год назад

      It depends on the bug, and pairing teams would decide whether to work on the bug alone, separately, or divide the work between them before getting back together to decide on next steps.
      This is engineering, not religion, the idea is to do what works. It is an illusion that pair programming is a less efficient way of working. Studies of pairing fairly consistently agree that 2 people working together complete the same task as a single person in around 60% of the time of the single person, so person for person, fairly similar levels of productivity, but they also produce significantly higher-quality work, which saves time on bug fixing, problem determination, going slow in future because your code is worse etc etc etc. There is no concrete data specifically on the subject of pairing of the impact overall of these less obvious wins, but the data for associated practices, like CI, CD and the associated ways of working has good data. Teams that practice these things spend 44% more time on new features than teams that don't. This is useful because this is the impact of working with higher-quality - the data is clear on that. So we can say "Working with higher-quality is more efficient" and we can say "Pair programming is one route that improves quality" so we can guess that pair-programming is more efficient than not. That happens to agree with my subjective experience too. The most efficient teams I have worked on did pair programming.

  • @arccth
    @arccth 2 года назад +47

    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.

    • @pee-buddy
      @pee-buddy 2 года назад +3

      Pair programming is still a work in progress

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +23

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

    • @edwardharman1153
      @edwardharman1153 2 года назад +12

      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.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +3

      @@edwardharman1153 That is certainly closer to my experience too.

    • @hypenage2415
      @hypenage2415 2 года назад +1

      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

  • @vimaximus1360
    @vimaximus1360 2 года назад +1

    can we get the links to the studies you mentioned?

  • @FiniteSteakMachine
    @FiniteSteakMachine 2 года назад +12

    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.

    • @MrTact67
      @MrTact67 2 года назад

      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?

    • @BittermanAndy
      @BittermanAndy 2 года назад +1

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

    • @jrhodes69
      @jrhodes69 Год назад +1

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

  • @WildfireS1
    @WildfireS1 2 года назад

    @8:28 One way around that is to integrate continuous integration into the pull request process. Where I work, we (generally) don’t use feature branches, but instead have forks from the main repository to a personal one, and forks are automatically kept in sync.
    We also have CI integrated into pull requests, so that each PR can be required to have a passing build in addition a certain number of code review approvals. While an individual is “waiting for a code review/approval” they are also simultaneously answering the question “does my change work with everyone else’s?”.
    That said, when I have been the lead or senior dev in my team, I’ve also taken the habit of doing pair programming/code review regularly, often before a pull request is opened. The result has been that when it’s time to do actually review and merge a pull request, the review part is relatively short.
    The downside is that I may not spend as much time as others on coding. I personally don’t mind that, but it’s not for everyone.

    • @ddanielsandberg
      @ddanielsandberg 2 года назад

      CI is not a "build server".
      CI is not "running checks on your feature branch".
      CI is not "rebasing/merging into a feature branch".
      Can everyone just stop redefining CI to be "whatever we're doing with branches right now"!?

    • @WildfireS1
      @WildfireS1 2 года назад

      @@ddanielsandberg would you like to share what definition you are using?

    • @ddanielsandberg
      @ddanielsandberg 2 года назад

      ​@@WildfireS1 I would start by reading the Continuous Integration article on Wikipedia.
      CI is a practice and a social contract. Unless the changes are integrated into mainline multiple times per day, triggering a build, linting, checks and tests, and output a versioned deployable "artifact" (or at-least a tag), preferably built **once** (as in opposition to have different builds from different branches for different environments) - then it's not CI.
      Jez has a very simple description here on youtube watch?v=IvWr29afDF8&t=270s

    • @WildfireS1
      @WildfireS1 2 года назад

      @@ddanielsandberg Thanks for elaborating. We have the same definition. I wasn’t asking because of uncertainty in my own understanding, but because you seemed to make assumptions about my original comment.
      When I mentioned “CI as part of the pull request”, I meant exactly that. Our build system will take the PR and create a temporary merge of developers’ fork (not a branch) with mainline at the moment the PR build is triggered and run all tests, static code analysis, and whatever other checks are configured. This will give the developer the same feedback as a true merge to mainline while reducing the risk of merging something that “breaks the build”.
      Once PR is approved with a passing build, it can then be merged to mainline where the same tests/checks/etc are run again. And for some products we have also integrated an automated deployment to a non-prod environment. But that part is maybe more CD than CI.
      Anyway, this kind of system is not feasible for everyone. My point was that there are other ways to reduce the negative aspects of pull request workflows as described in the video while answering the question “does my change work with everyone else’s” quickly. It doesn’t necessarily require abandoning pull requests. Take care.

  • @hanabimock5193
    @hanabimock5193 2 года назад +3

    Maybe I am too inexperienced but this is nonsense for me. The one argument is the PR are slow. Well that depends on the set up an amount of people reviewing. Also one can and normally has something else to do. So I will not just be there waiting to get an approve...

    • @coderider3022
      @coderider3022 2 года назад

      Yes, Sprint team should know they key items and PR for them is priority. I worked in a team where the reviewer was allocated during planning and it worked well.

  • @alejandroexposito9806
    @alejandroexposito9806 6 месяцев назад

    What about not waiting until the pull request to clarify the architecture, requirements, intent etc?? I have found that pull requests become the moment to clarify all that was not clear in the user story etc…. How does a developer know he finished his work?? The problems are present before the pull requests.

  • @SteinGauslaaStrindhaug
    @SteinGauslaaStrindhaug 2 года назад +10

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

    • @names-mars
      @names-mars 2 года назад +1

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

    • @SteinGauslaaStrindhaug
      @SteinGauslaaStrindhaug 2 года назад

      @@names-mars what doesn't happen often? Trusting people or small pull requests?

    • @Stettafire
      @Stettafire Год назад +1

      ​@@names-mars Same. Our PRs are generally about five classes as a rule. They almost always generate useful discussion.

  • @MaxCoplan
    @MaxCoplan 2 года назад

    At 8:30 you mention "I have to wait until my PR is reviewed and merged until it can even start Continuous Integration". Maybe I misunderstand what you mean by "CI", because to me "CI" means all the automated tests and test-like things. These tests can run as soon as the PR is opened and in parallel with any reviews. What did you mean by "Continuous Integration" or did you mean to say "Continuous Delivery"?

    • @stragerneds
      @stragerneds 2 года назад +1

      You're thinking of CI tools. Farley means Continuous Integration, which is about **integrating** (merging) your changes into the current/latest version.

    • @MaxCoplan
      @MaxCoplan 2 года назад

      @@stragerneds I see. I thought that running my tests against the merged result of trunk and my PR counted as CI, but it’s not about continually merging trunk into my branch, it’s about continually merging my branch into trunk. Thank you 😊

    • @MaxCoplan
      @MaxCoplan 2 года назад

      Am I uniquely stupid or is “CI = automated tests per commit” a common misconception?

  • @GDScriptDude
    @GDScriptDude 2 года назад +9

    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.

  • @marcotroster8247
    @marcotroster8247 2 года назад +1

    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

  • @aram5642
    @aram5642 2 года назад +11

    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.

    • @rand0mtv660
      @rand0mtv660 2 года назад +4

      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.

    • @andrealaforgia7699
      @andrealaforgia7699 2 года назад

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

  • @SeraphicRav
    @SeraphicRav 2 года назад

    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?

  • @magne6049
    @magne6049 2 года назад +12

    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.

    • @BementalSea
      @BementalSea Год назад

      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.

    • @magne6049
      @magne6049 Год назад

      @@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?

    • @BementalSea
      @BementalSea Год назад +1

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

  • @yesnickcarter
    @yesnickcarter 2 года назад

    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.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +3

      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.

  • @centerfield6339
    @centerfield6339 2 года назад +10

    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.

    • @Stettafire
      @Stettafire Год назад +1

      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.

    • @centerfield6339
      @centerfield6339 Год назад

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

  • @mnadjp
    @mnadjp 2 года назад

    I would really like to see the data points that you're talking about. Please share link.
    Specifically I want to know how many of these data points are taken from software consulting companies vs software product companies.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад

      The links are attached to the video, usually shown "underneath" but depends on how you are watching it. Most of the data comes from the "State of DevOps Report" from DORA and a description of how this was collected and analysed, as well as the implications of this research is all captured in the book "Accelerate"
      "Accelerate, The Science of Lean Software and DevOps", by Nicole Fosgren, Jez Humble & Gene Kim amzn.to/2YYf5Z8

  • @leokeuken9425
    @leokeuken9425 2 года назад +16

    Pull requests are still valuable for extra events and hooks to hang automation off of when following git flow. Also gives opportunities to stop bad code from being merged and easier roll back. So I do concur on pull requests not necessarily being reviewed properly, but there is still plenty reasons to use them.

    • @nextlifeonearth
      @nextlifeonearth 2 года назад +1

      It's like saying turn lights are useless, because people don't always use them and even if they do they don't prevent all accidents.

    • @andrealaforgia
      @andrealaforgia 2 года назад

      PRs are not strictly necessary for automation. The automated steps you attach to a PR can live in the build and deployment pipelines. What stops bad code from being merged is automated tests and code review, not Pull Requests. Code reviews can be performed in many different ways, for example continuously, when adopting social programming approaches. The more you dig down, the more you discover that no, there isn't plenty of reasons to use them. PRs were never meant to be used by durable product teams.

    • @andrealaforgia
      @andrealaforgia 2 года назад

      @@nextlifeonearth PRs are not lights.

  • @rakotoarivelotojo5071
    @rakotoarivelotojo5071 2 года назад

    In my team we really see advantage of pair programming, not for the junior + senior concern but more "if it's not simple don't do it alone, you will waste time".
    But in another hand, we are a very small team(less than 5) and work in remote, it's fairly not possible to practice a pair programming all the time. In that case, what should be your advice to avoid pull requests but still ensure a good code review?

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад

      First I'd maximise the pairing, do that most of the time, when most people are available, and then informally review changes that happened on the, hopefully fewer occasions, when you weren't pairing. I you normally pair, most people will be aware of the lack of review, and so probably happy to do a quick review when you need it.

    • @rakotoarivelotojo5071
      @rakotoarivelotojo5071 2 года назад

      @@ContinuousDelivery " and then informally review changes that happened", That was something I did before when code review was not well done on the team. But it's exhausting. When you present your review to the one who coded the feature, he says "OK I will do it later, I'm on something else" , but the "later" never happen because there is always something to do. It finally creates so much technical debts. And sometimes I changed the code myself and present it to the guy and discuss for a while about it. It maybe some (weird) variant of reviewing the code together but, in the end, it detach people of the responsibility of delivering quality. So we ends up with a more formalist approach (the pull request) that "force" people in some way to make review, and to deliver better code.
      We first tried we a Gitflow and it was such a non intuitive so now we are more on a Github flow. Even it's "better", as you said, it not really permit to have a CI.
      I wonder if there is a way to achieve a CI but ensure a good code review without pairing

  • @TheNiters
    @TheNiters 2 года назад +4

    Your argument seems a bit flawed when claiming you need to poll 50 000 developers to figure out what the best development approach is. I would never be looking for the "objectively best" approach to development, I would be looking for the development approach that works best for my specific team, so while evaluating the input from research like Google's DURA report can give you insight, the only really important people to ask are the people you work with.

  • @Masterche18
    @Masterche18 2 года назад

    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.

  • @PlatinumDragonProductions999
    @PlatinumDragonProductions999 2 года назад +6

    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 :-)

    • @PlatinumDragonProductions999
      @PlatinumDragonProductions999 2 года назад +1

      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.

    • @evgenyamorozov
      @evgenyamorozov 2 года назад +1

      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.

  • @mrpocock
    @mrpocock 2 года назад +1

    Good programmers and good programming teams make good code. I can't help thinking that the main reason some methodologies work is that they are what good programmers and teams of good programmers do. That doesn't mean the methodology is improving anything - it is just a really reliable indicator variable for good programmers.

  • @jonathansaindon788
    @jonathansaindon788 2 года назад +8

    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.

  • @devagr
    @devagr 2 года назад +1

    I would love to do this at my workplace. But there is a problem which is pretty unique to us, which is that we constantly recruit new and inexperienced students to work on the projects. This model inherently prevents building trust that the developers can commit code without first reviewing. Do you have any suggestions in this scenario?

    • @alessandro3974
      @alessandro3974 2 года назад +1

      Why not use pair/mob programming?

    • @uome2k7
      @uome2k7 2 года назад +3

      If depending mostly on juniors, pairing them up won't help too much.

    • @reav3rtm
      @reav3rtm 2 года назад

      PP is workflow that works where there is trust (experienced teams utopia). In real development environments you need to slow down the process because there are always newbies to the project or people who repeatedly commit unmantainable code that happens to work - in ex template metaprogramming masturbations. Pair such people together.. Pull requests are good way to deal with either. And when you set up PRs with automated tests, you get the best of both worlds, that is more applicable in real, than to utopia environment. My 5 cents.

    • @BementalSea
      @BementalSea Год назад

      Mob programming really helps in situations like this. It's a great way to help the new people learn fast.

  • @HedleyLuna
    @HedleyLuna 2 года назад +6

    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.

    • @moodynoob
      @moodynoob 2 года назад +3

      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.

    • @bitchain
      @bitchain 2 года назад

      What about the risk of an echo chamber meaning a review is not necessarily as extensive as new eyes

  • @plnmbjj
    @plnmbjj 2 года назад

    How do we advocate for trunk based development when someone says that it decreases the code quality?
    My team has been doing trunk based for the past 3 months and a lot of things improved, but one of the most seniors guys that approves prs all over some of the micro services that we have advocates against it because he wants to ensure code quality during reviews…
    Any suggestions on how to advocate for it?

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад

      Well the data says the reverse,
      The DevOps handbook says:
      “The data from Puppet Labs’ 2015 State of DevOps Report is clear: trunk based development predicts higher throughput and better stability, and even higher job satisfaction and lower rates of burn-out”
      Gene Kim, Jez Humble, Patrick Debois & John Willis
      amzn.to/2qV3SrZ
      State of DevOps Report, 2016 says:
      “We have found that having branches or forks with very short lifetimes (less than a day) before being merged into trunk, and less that three active branches in total, are important aspects of continuous delivery, and all contribute to higher performance. So does merging code into trunk or masted on a daily basis”
      puppet.com/resources/whitepaper/2016-state-of-devops-report
      So the "senior guy" is wrong. The problem is how do you convince that person?

  • @tylovset
    @tylovset 2 года назад +14

    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.

    • @Rodhern
      @Rodhern 2 года назад

      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.

  • @kdietz65
    @kdietz65 2 года назад +2

    In a large organization, pair programming doesn't work. Managers don't support the practice, developers don't like to do it, and the hodge podge mixture of different coding styles in a large code base makes it really difficult.

  • @TheDestino8
    @TheDestino8 2 года назад +5

    Something I'd like to see is a study that compares pair programming with two people working together remote or in person. Personally I've experienced both, but with different teams, so I can't really say if it's a problem in and of itself or just an issue with the constellation, but in my experience it's been a lot harder to do decent pair programming over the internet, compared to in person. So it would also be quite interesting to know the matrix between individual programming at the company vs remote vs the two variations of pair programming.

    • @ContinuousDelivery
      @ContinuousDelivery  2 года назад +4

      I haven’t seen any studies on that, but I have a fair amount of both too. Obviously it is a big easier when people are together, but I have found remote pairing is not all that different.

    • @KManAbout
      @KManAbout 2 года назад

      I've only done pair programming remotely so I can't say to be honest. It feels normal I guess because of this. There's a lot of software that facilitates collaboration. I jump into a teams meeting or whatever and discuss.

  • @michaelslattery3050
    @michaelslattery3050 2 года назад +2

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

    • @a544jh
      @a544jh 2 года назад +2

      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.

    • @michaelslattery3050
      @michaelslattery3050 2 года назад

      Right. The 2 key features are feature branches are always up-to-date with develop, and develop is basically PR queue for CD.

  • @victorlongon
    @victorlongon 2 года назад +4

    Pair programming is not a substitute for PR's. PR work great, usually the problem is the size of it.