I’ve Found Something BETTER Than Pull Requests...

11 місяців тому

Pull Requests are probably the commonest way to organise code reviews, but they present problems that get in the way of Continuous Integration. CI is ultimately more important in our goal to create “Better Software Faster” than code reviews, so this is a problem. So what could we do instead? What works better than PRs and bypasses the downsides of delays, asynchronous and box-ticking, low-quality, reviews? Pair programming is one very good option, but there are others that are a better fit for some teams and organisations. So how should we organise code reviews for big companies and small and what makes a good code review?
In this episode, Dave Farley author of “Continuous Delivery” and “Modern Software Engineering” and expert in software development in general, describes what works better than PRs and keeps the changes flowing to keep CI fed. He describes an approach called “Non-Blocking Code Review” and also explores how we can sensibly compare ideas like these without resorting to personal bias.
-
⭐ PATREON:
Join the Continuous Delivery community and access extra perks & content!
JOIN HERE for as little as £2 ➡️ bit.ly/ContinuousDeliveryPatreon
-
🎓 Continuous Delivery Fundamentals Course:
Get introduced to the concepts, benefits and key practices of Continuous Delivery, and develop your knowledge of the first steps to start building better software faster. This course is IDEAL for people who are new to Continuous Delivery and non-technical people who want to understand more about CD. Study the fundamentals of Continuous Delivery FOR FREE HERE ➡ courses.cd.training/courses/c...
-
🔗 LINKS:
“Optimizing the Software development process for continuous integration and flow of work” ➡️ itnext.io/optimizing-the-soft...
“Continuous Code Review” ➡️ trunkbaseddevelopment.com/con...
“Non-Blocking Code Reviews”, Thierry De Pauw ➡️ thinkinglabs.io/articles/2023...
-
BOOKS:
📖 “The DevOps Handbook” Gene Kim, Jez Humble, Patrick Debois & John Willis ➡️ amzn.to/2qV3SrZ”
📖 “State of DevOps Report, 2016" ➡️ puppet.com/resources/whitepap...
📖 Dave’s NEW BOOK "Modern Software Engineering" is available as paperback, or kindle here ➡️ amzn.to/3DwdwT3
and NOW as an AUDIOBOOK available on iTunes, Amazon and Audible.
📖 The original, award-winning "Continuous Delivery" book by Dave Farley and Jez Humble ➡️ amzn.to/2WxRYmx
📖 "Continuous Delivery Pipelines" by Dave Farley
Paperback ➡️ amzn.to/3gIULlA
ebook version ➡️ leanpub.com/cd-pipelines
NOTE: If you click on one of the Amazon Affiliate links and buy the book, Continuous Delivery Ltd. will get a small fee for the recommendation with NO increase in cost to you.
-
CHANNEL SPONSORS:
Equal Experts is a product software development consultancy with a network of over 1,000 experienced technology consultants globally. They increase the pace of innovation by using modern software engineering practices that embrace Continuous Delivery, Security, and Operability from the outset ➡️ bit.ly/3ASy8n0
Sleuth is the #1 most accurate and actionable DORA metrics tracker for improving engineering efficiency. Sleuth models your entire development cycle by integrating with the tools you already invest in. You get a full and accurate view of your deployments, see where true bottlenecks lie, and keep your team’s unique processes and workflows. With accurate data, Sleuth surfaces insights that your engineers can act on to improve - with real impact. ➡️ www.sleuth.io/
IcePanel is a collaborative diagramming tool to align software engineering and product teams on technical decisions across the business. Create an interactive map of your software systems and give your teams full context about how things work now and in the future. ➡️ u.icepanel.io/1f7b2db3
Tricentis is an AI-powered platform helping you to deliver digital innovation faster and with less risk by providing a fundamentally better approach to test automation. Discover the power of continuous testing with Tricentis. ➡️ bit.ly/TricentisCD
TransFICC provides low-latency connectivity, automated trading workflows and e-trading systems for Fixed Income and Derivatives. TransFICC resolves the issue of market fragmentation by providing banks and asset managers with a unified low-latency, robust and scalable API, which provides connectivity to multiple trading venues while supporting numerous complex workflows across asset classes such as Rates and Credit Bonds, Repos, Mortgage-Backed Securities and Interest Rate Swaps ➡️ transficc.com

КОМЕНТАРІ: 345

@ContinuousDelivery 11 місяців тому

Get introduced to the concepts, benefits and key practices of Continuous Delivery, and develop your knowledge of the first steps to start building better software faster. This course is IDEAL for people who are new to Continuous Delivery and non-technical people who want to understand more about CD. Study the fundamentals of Continuous Delivery FOR FREE HERE ➡ courses.cd.training/courses/cd-fundamentals

@Sergio_Loureiro 11 місяців тому

What is spaghepticism?! 21:00

@georgehelyar 11 місяців тому

@@Sergio_Loureiro when you're not sure if it's spaghetti code or not :D

@manishm9478 11 місяців тому

Dave starts explaining the technique at 17:25. My summary: - team must use trunk based development with continuous integration - code is written with small commits and merged to trunk when complete, then the ticket is moved to a 'ready for review' column on the team's kanban board - code is reviewed when someone is between pieces of work at the start of the day - unreviewed code may be released, however the CI process provides confidence that the system will still work

@woo545 11 місяців тому

Didn't get to that point, but no mention about Feature Flags?

@WarconNet 11 місяців тому

Thank you

@lmao6 10 місяців тому

I think this process could work for some but not all. Depending on your product, ci cannot catch everything. There have been many times where my team mates have helped me catch something before I merged /released. Having a phased rollout I think is really important. Use your user base as part of your regression tests instead of trying to ship perfect code.

@thomascking 10 місяців тому

Step four: unreviewed code is released. Step five: no-one reviews it the next day because they're being stamped on by the business to "deliver" features.

@cronnosli 9 місяців тому

I have a strong believe that if people want to work with trunk based development, they should stop using GIT and go back using SVN.

@JeremyConnor 11 місяців тому

Our code reviews (as pull requests) also find gaps in test coverage due to developer inexperience. So, we can't safely argue that the tests contained in unreviewed code provide us the surety of finding/preventing bugs, because there won't have been any oversight to ensure gaps don't exist in the added tests. I have worked with engineers who are so experienced that you could safely drop code reviews, however that is not the majority of engineers; if you have any team members who aren't in the top tier of experience and ability, then allowing them to commit to master and calling that releasable as far as I can tell, sets you on a path where it is a matter of time until you break production because they release a bug alongside test code which insufficiently covered the bugged code in question.

@ralftaraschewski6094 11 місяців тому

Totally agree. Also If you are a single developer, those merge request are gold to review yourself.

@riccanov 11 місяців тому

most companies also run automated quality checks that run as part of PR process automatically. If you pair, you pair is also human it does not catch everything, you can easily forget to run the necessary checks before committing, and git hooks don't always work well with any build tool (works ok with husky / does not work well with gradle for example)

@valtsmazurs4056 11 місяців тому

Test coverage could be verified by an automated pipeline which should mark the code as unreleaseable if the coverage is not enough. It would not check the quality of the tests, though, unless you'd use mutation tests but those tend to be slow. I still feel safer if the verification pipelines fail before my code is the main branch, therefore short-lived feature branches and PR's still seem a good option to guarantee that: 1) main branch contains only releaseable code and can be deployed immediately 2) I don't always have to run the whole test suite locally before pushing my changes. If I missed somethiong, I get the feedback from automated pipelines before my changes affect other teammates. I'm aware that it might be the old habits speaking, though :)

@alfbarbolani 11 місяців тому

I have seen places where they try this. The problem with those “small changes” continually merged into prod is that over time code becomes a pile of patches one on top of another , each managing to hit its target but not taking care of the overall code layout. The better the team, the longer it takes, but eventually everyone justifies these hacky approaches saying that they cannot refactor the whole thing and the thing needs to be rewritten from scratch. But even in top teams, after two years of practicing this, the mess is unsalvageable. Simple problems need simple fixes: if you are afraid of big merges , simply merge your branch with main everyday.

@dflor003 11 місяців тому

@@ralftaraschewski6094 +1 to that! I frequently review my own PRs (whether I'm the sole developer or not) to make sure everything is as I intended to check in. I've caught many potential bugs and accidental file check-ins like this.

@JamesSmith-cm7sg 11 місяців тому

Who's writing the automated tests though? If nobody is reviewing the tests you don't know if the tests are satisfactory. If the tests are wrong or missing cases you're immediately in trouble without reviews. You mentioned the data says pull requests are worse than trunk based, but who are the subjects of this data? High level teams with top talent might be able to release without review, but in many companies it won't work unless someone expirenced reviews the code. Pair programming is at least doubling the cost of a dev team. As you already said, companies aren't going to do it. The data isn't strong enough.

@sixstring896 10 місяців тому

I was just on a project at a US Fortune 500 company that is actively doing pair programming. I had never seen it in action before. Now I'm in favor of it. The review process happens concurrently with the development. Yes, it can interrupt 'flow' and it takes some learning. But I was impressed with the results.

@AndrewWalkerSyneil 10 місяців тому

I also want to know what this data is. Can you provide your source @ContinuousDelivery?

@vladogir5168 8 місяців тому

"Who's writing the automated tests though?" - There are a number of ways of addressing your concerns, for example, you can have automation setup to flag code without tests and possibly use something like AI to help more junior developers write code. The goal here is to keep code changes small so that it's much harder to miss possible cases. "High level teams with top talent might be able to release without review, but in many companies it won't work unless someone expirenced reviews the code." - Why would this not work in many companies? Do you mean that many companies don't want to change, if that is the case then it's a totally different question. The question I'd ask is have you tried proposing this change? In practice changes like this are difficult to introduce and require a lot more (and I mean a lot more) than one meeting with the stakeholders and may take months or years. "Pair programming is at least doubling the cost of a dev team." - There are various papers that state that you do lose output when pairing, about 10% as far as I remember, but you do gain a substantial amount of quality (and other benefits) as a result. This goes back to introducing changes in the company, which is not easy.

@brianmulder4920 4 місяці тому

Pair programming is a makeshift and outdated reaction to rapid growth and change in technology. It's an adhoc tactic, avoiding investment in mentoring, training, and distributed engineering practices. My two cents worth :)

@ForgottenKnight1 Місяць тому

"Who's writing the automated tests though?" - the engineers that develop the feature.

@thought-provoker 11 місяців тому

The data about post-hoc inspection has been around for decades: 1) Cost of Delay 2) Additional labour cost 3) Defect rates increase with amount of inspection. So Pull Requests "solve" (ehum) one problem, and thereby institute three new problems. Back in my Six Sigma days, we always used to say that "an ounce of prevention is worth a ton of cure," i.e. if by any means you can improve in-process quality, you can reduce the cost of poor quality by orders of magnitudes. PR's only have a place where you don't have control over in-process quality, i.e. open source projects with unknown contributors. In all other situations, in my opinion, PR's are a sign of QA not doing their job (i.e. figuring out why things go wrong in the first place, and initiating improvement there.)

@ddanielsandberg 11 місяців тому

This is an awesome answer and mirrors what I've been trying to say for years but lacking the language to do so. (commenting to have a bookmark)

@renegadosPL 10 місяців тому

I think the code review process solves the wrong problem. Usually we have this gate, because we don't trust people, or we are not sure if they produce good quality code. The better solution is to teach people how to write good code. My team does this through pair or ensemble programming and regular coding dojos for the whole team, where we learn how to break a problem into very small chunks (usually with 5-7 minutes of work). We learn from each other, we learn how to communicate with each other, we share ideas, we discuss the code, the consequences of doing something, the architecture, etc. I think the discussion is one of the best learning tools. The next problem with code review is that you can always say, it's not my fault, those guys checked it and didn't see a problem. There is a lack of responsibility, and I think a lack of trust that less experienced developers will merge something stupid into main. Another thing I don't like is that code review is sometimes harmful. Imagine you have been writing code all day and the other people say you have to rewrite it completely. It's demotivating and cruel. Instead, we should write the good code from the beginning. My team does not always do things in pairs. We've allowed ourselves to push and release code without someone else checking it, and we haven't had a bug in over a year. I trust my colleagues because I know how they work. They use TDD, they write tests first, then code. I know that if anyone has any doubts, they will pair up with someone else. Long story short, you should build in quality into the process and build trust.

@bernhardkrickl5197 11 місяців тому

This non-blocking review strategy definitely seems interesting to me. I feel tempted to introduce it and use it as a back-door to getting us to actual pair programming. :)

@underdog578 2 місяці тому

Great video, thank you Dave. Identifying that anything that introduces a gate in the software delivery process carries a cost of blocking a continuous flow and we should think if there are better ways to achieve the outcome. Code reviews definitely fall into that category. Some comments correctly point out that code reviews do catch issues like security holes, incomplete test coverage, missed use cases that will affect quality. I would argue that if you are currently relying on code reviews to catch these, it is already late. They should be caught before the code is written.

@petersurda6206 11 місяців тому

I don't see how working on the same branch magically avoids merge conflicts. Eventually all the changes need to be merged. Instead, merge conflicts are reduced by keeping each change small, irrespective of whether you use PRs or trunk based development. Since I can't use pair programming (open source, different time zones), what I do is if the PR gets too big I tell the author to split it into multiple PRs, and try to have enough work queued up so they can continue doing something else while waiting for a review. Also I tell people to regularly rebase, not merge, the main branch.

@BryonLape 11 місяців тому

His only answer is "buy my book".

@ricardoamendoeira5689 11 місяців тому

If you split something into multiple PR's that depend on each other and then someone suggests a change in the first one you now have to rebase all the other ones which is annoying and wastes time. Rinse and repeat for each other PR until the last one. And the longer it takes you to merge the first PR the more out of date with trunk all the other ones get. What he advocates for is yes, split the work into small parts, but commit each part to trunk instead of waiting for reviews in a PR (he advocates for pair programming so that each commit is already reviewed anyway).

@petersurda6206 10 місяців тому

@@ricardoamendoeira5689 I understand, I tried pair programming and I actually like it and see how it can improve the process, but I can't use it since there is not enough overlap of time availability. I also understand it's better to preempt a PR from growing big instead of telling to split it later. But again I don't always have these options.

@aaronhamburg4428 11 місяців тому

Just a thought about merging before code review, while I fully agree that the purpose of the review is to ensure code quality and not to catch bugs there could be other potential issues. In my experience sometimes more junior developers may introduce potential security risks with their code and if those make it to production this can be catastrophic. Security risks often can not be caught by tests or QA, only a pen test will discover it or if you are unlucky an actual attack. I guess junior developers can be assigned as not trusted and as such will not be allowed to merge without a review or have to always do pair programming but this comes with its own challenges. Such a mistake is not also reserved for juniors, anyone can introduce security risks without noticing. I am not trying to bash the idea, just some perspective

@mrspecs4430 11 місяців тому

Labeling juniors from the start as 'not trustworthy' will significantly damage confidence and motivation. They will have a hard time trusting their own abilities and won't share their thoughts on the project ultimately preventing them from becoming a good developer. This sort of elitist thinking is what causes a lot of problems in software development which I think Dave is trying to prevent with his approaches.

@aaronhamburg4428 11 місяців тому

@@mrspecs4430 Yes I agree, this is why I wrote "this comes with its own challenges". segregating the developers like this is very bad for the team as a whole and could hinder their own development. More experienced devs are also not immune from making mistakes as well. My point is that nothing comes without some side effects, second pair of eyes in a code review is quite good for everything else except that it's inherently a bottleneck. When do away with it we increase the the speed of development but we are exposed to more risk, saying that we are not is simply not true. In a sense, like with everything else, there is no one superior model that is the "best" for all teams and companies. I think people should look at the options at hand and the implications and choose what fits them best as opposed to follow blindly some model of working

@aaronhamburg4428 11 місяців тому

@@mrspecs4430 On a side note, labeling junior devs as 'not trustworthy' will probably have the effect you are describing for some of them and the exact opposite for others depending on their personality because some people might push harder to achieve the worth status. We can't know for sure just by guessing, as this will be pure arrogance.What I do know is that it will be an environment that I wouldn't want to work in.

@vyli1 3 місяці тому

@@mrspecs4430 whether you specifically label them as not trustworthy is not as relevant as how you end up treating them. And you will end up treating code from junior developers as less trustworthy, because quite frankly it simply is. It's only correct to make that assumption. That's why they're junior developers. If their code would be trustworthy, they wouldn't be junior devs, they'd be senior devs. The lower trustworthiness of their code is implicit to what it means to be a junior developer.

@philw3039 Місяць тому

@@aaronhamburg4428 I agree there is no silver bullet approach but it's important to re-evaluate our common conventions and recognize our own biases from time-to-time. For example, if security is that important to the project wouldn't a more robust solution be to: 1) Document common coding security anti-patterns and risks and make sure new developers are aware of them as part of the onboarding process 2) Require that all developers earn and maintain some kind of certification such as Security+ 3) Use code vulnerability scanning tools to ensure major security risks are identified. You could still choose to have PR's if that works for your team. Code reviews have several benefits. However, the notion that security risks are a "rookie mistake" and the only line of defense is a wise, hawk-eyed senior dev making sure the code stays pristine seems a bit biased to me. That strikes me more as something to ensure a pecking order than to truly benefit code security.

@adrianbilescu 11 місяців тому

Wow, nice to hear this idea. I just started implementing TBD since December and I found great value to keep a Code Review step but stil do CI. I'm so glad to hear this idea from you ❤

@theraven1979 11 місяців тому

For me the PR process is not merely to prevent bugs or ensure tests are passing but it's much more than that. I see a quality assurance aspect here that is far more important in terms of ensuring the code follows a common shared theme/style and approach (tools are available but it's more of a shared software team vision). As a Tech Lead I don't want to have to spend future days unravelling and refactoring someone's code that was poorly implemented from an architectural point of view (even if it is bug free). I'd rather shift left and identify any architecture creep early in the PR

@amyIsFlexable 10 місяців тому

I agree with a lot of the commenters who say that one thing code reviews catches is poor tests or no tests. But also this approach assumes you already have test coverage, which may not be the case, and I think this would be disastrous in that case. In addition, there's the cultural problem that product always wants more and more features and so if you have code that "works," you might have to fight pretty hard to get the opportunity to go back and make it better--even if you caught that it needed improvements right away. Having that approval gate lets developers conceal that process from the business so code quality can be maintained.

@brownhorsesoftware3605 11 місяців тому

Yes! Yes! I worked for a feature phone platform company that put all of its competitors out of business (before being acquired) by using a similar approach. We cut the development time for a feature phone from a year to 3 months. For the most part, we communicated through code - both locally and remotely.

@pchoudhary 8 місяців тому

While at Thoughtworks, we used to have Code reading sessions in my projects. So instead of reviews at PRs, we would go through the diff weekly once, entire team. And let the team think about what can be improved. And if something was worth improving, we would do it during next change. And everyone now knows and agrees to the need to change.

@tomeofkeep7842 3 місяці тому

That'd take days!

@ZapOKill 10 місяців тому

1 minute in to the video, and really curious what comes now. what a hook! Well done Dave!

@POINTS2 11 місяців тому

So true about code reviews being about code quality and not catching bugs. At my company, we used to do code reviews after code was checked into the mainline trunk. We have since then "upgraded" to using git PRs and now the changes are tested in our CI/CD pipeline much later than before.

@mecanuktutorials6476 11 місяців тому

It’s mostly to ensure the checked in code is maintainable by others going forward. Cleanup after checkin is unrealistic because there won’t be any time for that later.

@baka_baca 11 місяців тому

@@mecanuktutorials6476o true, when it comes to code clean up, later often means never

@ForgottenKnight1 Місяць тому

@@mecanuktutorials6476 "Cleanup after checkin is unrealistic because there won’t be any time for that later." - says who ?

@mecanuktutorials6476 Місяць тому

@@ForgottenKnight1 once it’s checked off as “done”. There’s always other stuff that is prioritized ahead of refactoring the older submissions.

@bernhardkrickl5197 11 місяців тому

Dave, thank you for your great advice! I just want to point out to others that feature-branch based code reviews can still be an improvement to lesser strategies. For us, it was. We have a bunch of developers but also a large code base, so we don't run into merge conflicts very often. Most of the time they are easy to fix. And when we merge, that very rarely breaks anything. While it's not perfect I think our amount and quality of testing is reasonable. One little thing that helps a bit is that the CI/CD platform we use always runs the build and tests on a temporary merge of the feature branch with the trunk. I know, that is also not optimal, but better than not doing it :) Yes, we can improve and we should sooner or later. But we came from a worse place.

@rj7855 11 місяців тому

I personally hate pair programing, It prevents me from ever getting into "the zone" ( Pair debugging legacy code is another story)

@FudgeYeahLinusLAN 11 місяців тому

Same. Very hard to get into those "10x moments" during pair programming.

@davidboreham 10 місяців тому

Pair programming imho is just a fancy name for "training a colleague".

@CyLvCompetetiv 10 місяців тому

@@FudgeYeahLinusLAN these 10x moments usually end in bad code because you end up coding for hours on end. Pair Programming gives you a framework to work on code sustainably and with a clear mind. You switch every 10-15 minutes and take a 5-15min break after you and your pairing partner had their turn. This is in my experience produces way way better code, removes the need for review & is much more sustainable aswell as a more sociable experience which is a plus if you have nice coworkers.

@zoladkow 9 місяців тому

I prefer pair reviewing, because all the back and forth with comments gets old really fast and is a place for festering passive-aggressivenes 😁

@edwardcullen1739 9 місяців тому

There are two states in which we *should* be operating: 1. Complete trust, so there's no need for pull requests. 2. Building trust, so we should be working very closely together to ensure we're learning each other's strength and weaknesses: i.e. pair programming. If you think about it, pull requests don't make a lot is sense, except for open source.

@GDScriptDude 11 місяців тому

I used to do the equivalent of pair programming in electronics engineering where I sat with a PCB CAD engineer and we worked on circuit board layouts together. The layouts came out great, but colleagues that did not do this had poor results with tracks burning up and bad component placement etc.

@LoftyCuber 11 місяців тому

Where I work I try to encourage pair programming and any code that is pair programmed automatically passes code review and moves past the review column in the Kanban board. While code that is not pair programmed is gate kept on a feature branch. I'm trying to move towards more and more pair programming. The issue I see with this is code reviewing an entire feature of small commits with other commits in between the ones I'm interested in would be difficult and it'd be hard to track their changes over time to know if they are good or not. Something may look bad only to get fixed in a later commit or look good and get worse in a later commit. Or someone else changed the class in between their commits. I've found that having any kind of code review tends to implicitly encourage larger commits as they are easier to review from looking at the git diffs. On a somewhat unrelated note. We have a very elementary pipeline set up and our unit tests take just a few seconds while our acceptance tests take around 15~20 minutes. Because of this I rarely run the acceptance tests on my machine or while pair programming and am nervous to commit to trunk directly as unfortunately we frequently accidentally break the acceptance tests due to oversights. Any advice on how to handle this? At the moment I commit to a feature branch, wait for it to pass and then merge to trunk later in the day.

@wal0x 11 місяців тому

I'm not an expert, but from what I understood you would accept the risk of potentially breaking trunk, but need to get build notifications and fix any issues that come up asap with highest priority to ensure it doesn't stay broken. And work to make the test duration shorter I guess.

@maximilianbeck2301 11 місяців тому

You could use a method from lean manufacturing. I think it was called the „andon cord“. Translated to the world of software engineering, it would mean: Make the state of the deployment pipeline visible to everyone in the team. (In the old „office times“, we had a screen in the office of our team showing the dashboard of our CD tool which was constantly refreshing - now I have a live version of the dashboard open on an external monitor or getting automated slack messages for status updates). If the deployment pipeline on the main branch turns red, you immediately coordinate how to fix it (mostly the author of the commit will take this or you pair/mob to resolve the issue). In the mean time the team could also work on „prioritizing“ the acceptance tests so that there is a suite of „critical“ tests that can be executed on the local machine in reasonable time and run all the tests including the ones for edge cases (or lower priority things) later in the pipeline. You could also try scoping the tests „by feature“ so that you can run acceptance tests of the things you’ve most likely touched on your machine in less time and run all the tests in the pipeline once the change is integrated.

@scotthinton4610 11 місяців тому

I've worked in aerospace for 8 years now on greenfield and legacy projects and I'll add my two cents. In my opinion, reviewing the requirements and tests before writing and committing code is absolutely critical. As others have mentioned, I don't think tests should be merged to a release branch without a formal review, whether it's a pull/merge request, or everyone sits in a room and goes through things step by step before the changes are pushed to the trunk branch. Adhering to BDD and TDD principles by specifying a feature and the test scenarios that describe it/prove it works as intended, then reviewing with all of the stakeholders should be done before any code is committed because ambiguity will be minimized for all stakeholders. For example, someone without knowledge of the domain or hardware systems the software is controlling may think some utility command is irrelevant and exclude it from the implementation while the operations engineer using the software assumes they'll be able to send that command because the component specification the software will interface with says the command exists. Once that baseline is established and agreed upon, then I agree with everything else you described here provided there is automation in place that couples/traces the execution of the tests back to the same requirements and test scenario artifacts that were previously reviewed and approved by the stakeholders, and that the trunk branch cannot be automatically deployed to a production environment without a final deployment readiness review. Of course, feature/test deltas need to be reviewed just as the baseline. Too often I have experienced the exact issues you describe which lead to unnecessary delays, increased cost, and poor quality software. When you have a seemingly limitless amount of capital and personnel to work with, the old Apollo method of developing software looks and sounds great, especially when the outputs of the process are good enough", but it certainly doesn't optimize for cost and isn't benefiting small space start-ups in these less economically forgiving times. Thanks for producing these videos Dave, extremely insightful stuff.

@mecanuktutorials6476 2 місяці тому

Honestly, for aerospace, you need to take it slow. The Pair Programming and Agile stuff is suited for non-mission critical software you find in regular corporations. IT/data stuff. Don’t follow this advice for firmware, especially safety-critical firmware.

@philw3039 Місяць тому

@@mecanuktutorials6476 Seems more like QA testing issue here more than a code-review issue. No matter how meticulous and thorough the code-review and automated testing process is, only manual QA testing can reliably determine if software is production-ready. Even 100% correctly written code doesn't guarantee software will work as intended. This goes _double_ for mission critical components. And while code-review can be a beneficial preventative measure to avoid potential issues early, when the process becomes too bloated it serves as red-tape that prevents getting timely feedback from proper QA testing. A rapid develop-QA test-rework feedback loop is arguably more valuable than extremely detailed code review. They are not mutually exclusive practices however. Ideally, they should complement each other. Unfortunately, many companies tend to double down on the red-tape aspect when things go wrong, trying to lockdown the process to prevent any and all mistakes within the code, instead of streamlining the process to get feedback quickly as possible.

@mecanuktutorials6476 Місяць тому

@@philw3039 I agree. Only manual testing can ever really proxy what the user will experience. In my experience, firmware development is closer to mechanical manufacturing in how it is handled. There is a stark difference between it and Desktop/Web/Mobile which are intended to run on multiple abstracted platforms. A lot of these other fields can “trust” the layer below works and covers ur behind. That trust extends to reviewing other’s work and lets things like Pair Programming happen. You only talk at a high level because the lower level stuff is trusted to work. But in firmware, things have to be more precise. Firmware is the kind of code that really does converge to a requirement. It’s usecase-specific and too costly to shift gears because it is so tied to the system/hardware. Code review is the only way to review details independently and provide precise detailed feedback or concerns. Code review during any development is indeed a hurdle because you’re just trying to make it function. Too much feedback and back-forths become an obstruction to further development. At some point though, the work has to be made presentable for others to read and maintain as well as give others an eye on the state of the codebase. Overall, pair programming or code reviews are not one size fits all tools. Trying to enforce them in all situations will diminish their value. But manual testing is inevitable.

@awesomesuprise9141 9 місяців тому

Many thanks for your materials!

@ashimov1970 11 місяців тому

I started my career in programming long (21+ yrs) ago doing programming in pair with a product manager. It was an incredibly inspiring experience that sparked my interest in IT career. This is why I strongly agree with you, Dave, and believe pair programming not only delivers great value but also provides a cool engineering experience.

@Meritumas 11 місяців тому

second it! Sadly I found myself in such situation only in 2 companies I work for... current gig is a disaster from dev processes.

@marna_li 11 місяців тому

I'm in a team using Kanban and Pull Requests for reviews. We have status meetings every morning to check the board. 30 minutes. Mostly to make sure that the throughput is constant - following the WIP limits. We constantly struggle with respecting them - since when work becomes a PR in the Review column the developer is eagerly looking for something else to do. I don't like this stress och pushing things through and feeling bad when stuff aren't moving on the board. That ignores the joy of software development.

@ddanielsandberg 11 місяців тому

Sounds like a bit of broken KanBan. Start by removing the "review" column and only have "doing" and "done" columns . Then set a WIP limit in doing. That way no one can start something new unless something moves out of "doing" and the only way to do that is to finish it. Then maybe the CI bit will start to click...

@salvatoreshiggerino6810 11 місяців тому

@@ddanielsandberg Or better yet, patch the Kanban software to not allow pulling from "to do" unless "review" is empty.

@Blob64bit 11 місяців тому

This must be a very common issue since I've also seen 2 projects like this. A lot of pressure and not productive at all!

@ForgottenKnight1 Місяць тому

Sounds like the objective on that project is to push items from left to right on a Kanban board, not make better software, faster. Kanban is a very good tool (used it for years) if you understand its objective.

@marna_li Місяць тому

@@ForgottenKnight1 OK! Great! I think some pick Kanban because it is simpler than "Agile" methodologies. Easier to explain what happens, what they supposedly get from it, and get everyone on-board. Leaves little room for complaints. The problem arises when you don't value quality as much as getting stuff done. Or when everything gets reduced to the item on the board, rather than what it is about. I dislike when someone talks about something as "just an item" in the backlog or the board.

@tristanmills4948 11 місяців тому

It is so frustrating when we have the goal of CD, but also have to use pull requests and code review...

@ForgottenKnight1 Місяць тому

"...have to use..." - who's forcing you ?

@user-ty7ig9yy1n 5 місяців тому

Hey Dave, right at the beginning you refer to some kind of data that states the correlation between process gaps and quality. I'd love to see that data. Could you give a hint where you found that? Thanks for your awesome content!

@br3nto 11 місяців тому

I agree with getting code out into prod asap to get feedback, but I’m still still not convinced on the No branch policy. Firstly, just pulling from remote creates a distinct local branch. But more than that, I’m usually working on a few ideas at once, swapping between them over a day. Or checking out other changes from other team members… I don’t know how you could do this without branches…. Maybe with git worktrees… but it’s kinda like working with another branch anyways… further more, doing this my team is still able to deploy multiple times a day… and after, we just rebase and continue. But granted, I don’t know how to get rid of the PR without the equivalent of pair programming before release. But do we need to review in person? Sometimes it’s definitively better, especially for complex changes.

@AbdullaFaraz 11 місяців тому

Honestly, this is the best channel of the kind.

@theisegeberg 11 місяців тому

I love this idea! I'll see if I can try it on an actual project. Best of two worlds.

@nurdauletturar 11 місяців тому

Huh, I had a similar idea a couple of months ago. The idea that we don't need to consider our software "releasable" only after it is reviewed and tested. We could trust our team members that they can write code that works. Using TDD, API-first design and such. It's just that the code might not be of the best quality it has a potential to. That's why reviews could've been done asynchronously or in a pair after the feature is working. Unfortunately, I haven't been able to try this idea in full, since I'm the only backend engineer in my projects. Plus, I didn't have enough courage to suggest making such a "radical" experiment.

@gerke_kok 10 місяців тому

It's always so cool to hear all these great approaches to getting quicker feedback and learn from each-other in the workplace. But I sometimes wonder how this will work in a 2 people team or a team of just-starting-juniors and only a few seniors or only one or none. Won't it be nearly impossible in these situations?

@ForgottenKnight1 Місяць тому

Pair programming by rotation. Depending on the size of your team, you might have to do multiple permutations before all seniors have worked with all juniors. There are plenty of videos on YT on how to do this.

@codingbloke 11 місяців тому

"We don't improve quality by adding bureaucracy" that should go on a T-Shirt!

@Nowwatchingyou3 11 місяців тому

Hey, Dave! I appreciate your videos and have read your book! I have found both to be very helpful! But I am hoping to get some advice on how do we get the team to that level. How can we get the team to be comfortable with the approach you propose in this video? Currently, my team follows a gitflow and pull request process. Exactly as you mention it in this video. The team is young. Almost everyone has 5-8 years professional experience. Everyone is aware of what a CD process should look like but none want to 'risk' following a new process they do not understand. How can I get them more comfortable with it? Keep in mind that I cannot force them to read your book and watch your videos 😅. As far as I know, convincing your team is a whole different beast.

@wal0x 11 місяців тому

Maybe you could try a Ship-Show-Ask approach as a stepping stone.

@maximilianbeck2301 11 місяців тому

„Everyone is aware of what a CD process should look like“ Then the work of convincing might be already done if the team agrees on roughly the same CD process and that they also want to implement this (this would be the first question - ask what they imagine to be sure CD is understood by everyone). Changing to CD is not done over night. So maybe discuss some small steps you could take as a team from the current state to move a bit closer to the desired state or process and investigate from there. From there you could improve further or roll back and try a different direction (as it is in agile product development). This might overcome the fear of trying something entirely new. The team might need some help from others (or management) to fulfill some prerequisites. For example the team would need the autonomy to deploy their software independently at some point. - This means the software needs to be testable and deployable independently. - This means the team needs to have the full authority to determine if the software is releasable or not. … and so on. You might need to convince a tech leader in the company to follow these kinds of goals (hopefully you don’t have to convince for these goals but my experience tells a different story 😅). So do changes in small steps and you’ll get there at some time. If you need help from a person with more experience in this way of working, asking questions (or for help) about these tiny improvements will also give a more detailed answer.

@TheEvertw 4 місяці тому

I recently worked in a project that had a design problem which I was trying to fix. But because of the pull-request & review based release method I had to merge the re-designed code three times, and wait on a review each time. This added four weeks to the release. And made me decide to leave the project...

@stephan553 11 місяців тому

Dave, could you adress how you would "solve" pair programming in a distributed (time and/or space) team, that for many good reasons agreed on asynchronous work? TY

@Kyocus 11 місяців тому

Feature branches should be rebased each time any branch is merged back to trunk. This ensures that all changes are integrated each time any change is ready to be pulled in. This branching and merging strategy integrates code as fast as it's merged, and as fast as any CI strategy. The thing which many people don't take into account is that there is inherent risk to pushing directly to trunk for the entire team, and time IS lost in editing the DAG to correct pushed mistakes.

@barneylaurance1865 11 місяців тому

This always comes up. If you do this and you have two feature branches created today, and neither of them get merged for a week then they are not integrated with each other for that week. I could delete a class in one branch and you could add properties to it in the other branch. Since neither of them is being merged the issue won't necassarily be detected. It would be worse if it was a more subtle incompatibility.

@valtsmazurs4056 11 місяців тому

@@barneylaurance1865 Potential merge conflicts can be solved by timely communication between the involved team members.

@georgehelyar 11 місяців тому

I can go with pull requests or pair programming, but I'm not convinced by review after release, where nobody has reviewed the code before it goes out. I think this is more likely to lead to bugs, and as we all know, bugs slow you down in the long run. Higher quality up front is faster. Code review is also a great time to break down knowledge silos, and for junior developers to learn. Also, the tests might pass but they might not be right. Even with TDD, people make mistakes. The tests might test one thing but actually it's not the correct behaviour because of a misunderstanding, or a junior developer might be new to TDD and add more than the minimum code required to make the tests pass etc. There's also performance. Performance tests are great to have but they can be slow to run, giving slower feedback, and it's quite common to catch performance problems in a code review before the performance tests. Where I work we do pull requests with a minimum of 2 reviewers, and the average completion time is about an hour. In that time, you can keep working on the next thing, so you're not really waiting at all, and you don't deviate too far from trunk. We can still deploy multiple times a day, but what we deploy is higher quality as a result. We do this by using a pull request dashboard for visibility, with desktop notifications (e.g. new pr, comment added, build failed), and a culture of checking what else needs review after submitting your own pull request and before starting on the next thing. This also allows static analysis, security scanning, test running etc to happen before the merge, so it's less likely to break trunk. If you cause a problem it only blocks that pull request, it doesn't block anyone else, and you get a notification immediately so you can fix it quickly.

@Blob64bit 11 місяців тому

Respectfully, the idea is not to eliminate code reviews. Quite the contrary, working like this should improve code reviews. As he said around 3 min mark this way is the only way where you can review the whole application state multiple times per day - as opposed to pull requests where you often review only one isolated part of the software (excluding other pending pull requests). I think the basic principle is to constantly push code to a production-like environment (not production, especially if you are building rockets etc. 🙂) and use it more broadly from users perspective, while also reviewing the code in pairs while talking etc. As he said -- this is probably one of the only ways to ensure that the changes work with each other multiple times per day. I find this also allows for people to work simultaneously on same / overlapping issues, which is always a hassle with gitflow

@dontdoit6986 11 місяців тому

He’s suggesting pair programming as the replacement for code reviews. Essentially, the devs are reviewing themselves.

@Blob64bit 11 місяців тому

@DontDoIt I got that he thinks pair programming is the best way of doing code review, not replacement. Having the person there explaining the changes is extremely helpful when reviewing

@georgehelyar 11 місяців тому

@@Blob64bit but if the code reviews are not a gate, it doesn't have to be reviewed at all before it hits production, which is the part I don't like. This just means it doesn't have to be reviewed at all. You have to assume that developers, including junior developers, never make mistakes that aren't caught by their own unit tests, but sometimes there are misunderstandings, or badly written tests etc. You also need to make sure that you do the reviews later and come back to improve the code, but this is just adding a lot of tech debt, and if you can't repay it fast enough, tech debt slows you down. Also, pressure for features often means that going back and improving things after delivering them is seen as a low priority by the business. As far as they care you delivered and now it's time to move on to the next thing.

@georgehelyar 11 місяців тому

@@dontdoit6986 he starts by saying he usually recommends pair programming (which I'm fine with) but then goes on to suggest an alternative approach in not blocking the release with code reviews, which I don't like as much, because it means code might not be reviewed at all before it hits production.

@n30v4 10 місяців тому

I find it realy challenging to do pair programming in small companies. We do CI/CD and it works realy well but we lack reviewing our code. Well I know the code "works" but reviewing is an important part to learn and have a foresight. We do this once on a friday and try to at least have a glimpse on what each developer has done. Normally this is not hard because we see the changes every day so we can discuss more broader topics. But, when we don't have time for that, because we need to jump in other projects, its like build a mountain with unknown changes. Each day we don't look into the code the mountain gets bigger and bigger. And when we meet on a Friday and didn't follow the changes, it takes a lot more time and the code is already deployed in production. And changes we should have done can't be done in time. We need a strategy to avoid this mountain or keep it small. We also lack in developers at the moment which does the whole thing harder. :)

@IliaFeldgun 11 місяців тому

Seriously I was looking for a take on code review from you

@rudiservo 11 місяців тому

Hi Dave, I do resonate with a lot of what you said for small closed teams and private development, and I do see a lot I personally can improve that can lower frustration and increase value. Now for the elephant in the room, it's not the same for Open Source projects, and currently validation has to be done in the form of pull requests, So things tend to be slow for outsiders of the core team to add code to the project, even high quality code. I don't think there is really a better alternative to that but I would love a video of yours commenting on it. Thanks for the videos.

@deanparker7867 11 місяців тому

Fantastic timing on this video! I'm currently acting as an enterprise architect for a very large and well-known company (I'm a consultant for an equally famous global company) and just this morning we were discussing code review and branching strategies for a specific application. I shared a link to one of your older videos on this topic. I can use this new video immediately as a discussion point. I agree with you completely and I'm glad you see other legit approaches in addition to pair programming. I'm working hard to develop an idea that bridges the gap between true pairing and non-blocking code reviews as at very large and older companies facilitating swift change on your personal timeline is a fool's errand.

@eus9 11 місяців тому

What data are you talking about when you say process gates correlate with poorer quality? Can you reference the study since it's one of the key axioms of the video?

@apacheaccountant9757 Місяць тому

14:12 Before merging feature to develop, develop branch with verified changes of other developers should be merged to the feature branch. After this CI verify all verified changes of other developers plus not yet verified changes of the feature branch. There are the settings in git providers that doesn’t allow to merge from outdated feature branch.

@GeraldOSullivan 4 місяці тому

Thanks Dave. I am borderline autistic so pair programming is not an option for me. The cognitive load of coping with someone else in my space overwhelms me, so Thierry's solution is an excellent alternative.

@coZhenya 10 місяців тому

Thank you for inspiring us to taking our processes to the next level! As you suggest, we also switched to trunk based development completely and we are happy with it so far. As a consequence, we do have a question about deploy version management: If our CI/CD server now creates a version bump automatically, we would need to also commit this change to VCS, essentially creating another automatic commit for each pushed commit. While this does work, it does not seem to me to be the best practice. I'd be curious to hear your thoughts / opinion. 😄

@ContinuousDelivery 10 місяців тому

I am pleased, but not surprised, that it is working for you 😊 There is no perfect answer to the version number problem, other than discarding 'semantic versioning' which isn't a great answer either, because it is useful to communicate with customers and users. My recommendation is use a generated unique id as the 'real version number' or 'release-candidate id' and treat the semantic version as a kind of 'display name'. You can keep a record associated the display name to the correct version number somewhere else and so not compromise the VCS of the release candidate. The problem with this is when you need to package the display name as part of a binary delivery. It is largely not a hugely risky thing to add it as a final packaging step, but I prefer the purity of 'test what you deploy into prod' if you can manage it. So a compromise, but not a horribly risky one.

@coDocumentary 10 місяців тому

@@ContinuousDelivery Amazing! Thank you Dave for the detailed response. 🙏🏽 (I'm in the same team as @coZhenya.) I love the idea of treating the "user"-needs (version number display) and "tech"-needs (unique-id) separately! I admire the consistency of your applied mindset and context-aware openness that I see reflected in your reply. => Why only use the single responsibility principle in code. 👏🏽 And yes, you anticipated correctly that I want to inject the version(s) into the build so users can supply that information for debugging purposes. I concur with your preference of bumping/generating the version(s) prior to testing to keep it pure/clean. Since I'm rather new to trunk only development: Would you be so kind to elaborate on your thoughts about the double-commit challenge? Is it perfectly normal to let the ci/cd pipeline commit the changes so that a future release is aware of the last version? Or was the idea that using a generated id replaces the need to "remember" the previous (non-user-dispaly) version? Thanks for sharing your intelligence about the ability to change. ☺💙

@ContinuousDelivery 10 місяців тому

@@coDocumentary My preference is to use the "real release candidate id" and only associate the "semantic-version-display-name" once the pipeline has approved the change. So I have done this, in the past, with a separate step and separate, tiny, stage that 'attaches' the version to the release-candidate. In one version we patched a file, designed for the purpose, with the display name version number. It depends too much on you tech to go much beyond that as advice. I think what you are looking for is the minimally intrusive way to 'attach' the display name to the code.

@coDocumentary 10 місяців тому

@@ContinuousDelivery Perfect. I appreciate your advice and it is enough for me to get the idea. I was a bit hesitant with patching the code at that point, however your strategy and the reasoning behind it does make sense to me and I feel more confident to commit to testing/adopting that approach now. 🥰

@RinkeColen 11 місяців тому

Question: you say there is a negative correlation between proces gates (like pull requests) and quality. Has this correlation been controlled for other factors like project size?

@gerke_kok 10 місяців тому

I was wondering this myself: does the level of experience for example go hand-in-hand with the felt need for extra gates? Or some other correlation.

@YisraelDovL 11 місяців тому

So if you don't use feature branches, that means that every commit is deployable? How do you commit early and often ?

@NachtmahrNebenan 10 місяців тому

With Home Office and less joint working time, code review or pair programming becomes really hard to establish. How to deal with that?

@br3nto 11 місяців тому

15:35 how is code “code hidden away on a feature branch” any different from code checked out locally and not pushed to the remote trunk?

@ddanielsandberg 11 місяців тому

Because we never hold on to changes for more than 10-15 minutes.

@mieszkometody 11 місяців тому

Dave, I love community comments and polemics under your content. Maybe it is a matter of not being a native speaker or but I understand and learn more from comments than from the actual presentation (visual side is great, but maybe sophisticated speach strictire or presented concepts are too general aka high level for me, a simple dev?)

@ltybc425 6 місяців тому

what you rebase your branch periodically to keep up with trunk?

@maxpaw 10 місяців тому

I agree with others who might wish to share this sentiment - the channel's weekly videos are lengthy and standalone, making it tough to keep up. Instead of eagerly waiting for Dave's content, I find myself questioning their value (even considering if they're worth my lunchtime). The view count compared to the subscriber base speaks volumes. I propose less complex but still insightful content, with more thematic connections for binge-watching. Avoid churning out filler content to meet a quota. A lesson learned from your own Cyberpunk 2077 video about managing time and scope. One value quality over quantity, a good video is more appreciated than weekly releases only you yourself seem to value. Thanks anyway for the effort, team Dave, loved the book.

@Tifredi 10 місяців тому

How would you advise to work in an environment such as a game development team where some people are working on a game and others are working on engine features or lower level features of gameplay? having worked with a CI oriented team in such environment, it could be challenging to have parts of the engine broken for several days when you were developing a gameplay feature... How to accommodate game designers that need a kind of "LTS" version of the tools they use with the need of continuously testing and breaking things of the engineers building the tools? Would you say CI is inadapted to game development?

@ContinuousDelivery 10 місяців тому

Well, sorry, but it isn't CI if there are "parts of the engine broken for days"! The aim is to make small changes that keep the SW working after every small change, commit them to check that these small changes work with everyone else's small changes and if one of your changes breaks something at that point, you fix it to revert the change immediately. CI is about committing WORKING SW multiple times per day, it is a more incremental way of working. CI and CD both work very well for game development. It takes a bit of adaption in the way that you work, and it sounds like the team you worked with hadn't done that.

@zerfahs 24 дні тому

How many times have I carried out a pull request code review and the code is not how I would want it but it works and re-writing would be too costly. Pair programming would have caught this at the earliest stage and there's your quality built in.

@baumstanz 10 місяців тому

Confuse correlation with causation. the quality gates might be there, because the quality has been bad in the first place.

@ContinuousDelivery 10 місяців тому

That is always a difficult problem in sociology. Hard to distinguish, but this does sound a bit like you are dissing the methodology, maybe by guessing what the methodology was? That isn't good science either. Have you read the book that describes the approach?

@baumstanz 10 місяців тому

@@ContinuousDelivery you're right, in that I'm just quoting you from the introduction without going through your sources. Which data are you referring to? The state of devops report? BTW I completely agree with your own methodology and conclusion, combining data and explanatory models. Just stumbled over the correlation part. I'm into the topic because I try to estimate the effect of pair programming in one of my own projects but I think it's confounded by the fact that only easy stuff is done alone - thus outperforming pairs in some way 😅

@JoelGilLeon 7 місяців тому

if/when you decide to switch out of twitter. would be great to follow your thoughts on mastodon.

@MegaMage79 11 місяців тому

I always wonder, and can't quite figure it out, is it possible to have different people write tests for your code and still do CI ? If someone else is going to write, let's say, integration tests, then there's always some code in trunk that has not been tested yet (but all tests pass because tests for new code don't exist yet). So then you can never trust your CI, because you never know if tests for all new code have been added yet. And even if you knew that, testing is probably always a bit behind development, so you can never release.

@ApprendreSansNecessite 11 місяців тому

Pull requests help track the pieces of code to review and make it possible to comment and all that jazz. Do you have suggestions for an alternate flow? The review column is what I would reach for instinctively and I guess semantic commits help a lot but doesn't it feel a bit messy / inconvenient?

@monarodan 11 місяців тому

Tools like Jira/Bitbucket will cross-reference commits to tasks. If it is the task that is progressing into a needing-review state then you can find the relevant commits directly from the task.

@ddanielsandberg 11 місяців тому

@@monarodan This is the way.

@ApprendreSansNecessite 10 місяців тому

@@monarodan This is also expensive, but I guess it depends on your situation

@monarodan 10 місяців тому

@@ApprendreSansNecessite why do you say it's expensive? It's literally just mentioning the ticket id in your commit message.

@ApprendreSansNecessite 10 місяців тому

@@monarodan my bad, I was looking at the wrong number of licences when checking Jira/Bitbucket pricing.

@nayanchoudhary4353 9 місяців тому

Love the shirt 😊

@cronnosli 9 місяців тому

PR's has no direct relation to quality, but to two other reasons. If you are using PR/MR for quality, then you are doing it wrong. The reasons are, first, the code been ready this happens because you need the team to be able to develop things without changing the behaviour of the mother branch, until you have finished your work, allowing that the system could be validated in its isolation(No other features intervention). The second one is related to the fact that not every developer is Senior. Companies have a lot of people on different maturity levels working on the code, so you need to review to make sure that code directives and patterns are been followed.

@ContinuousDelivery 9 місяців тому

They have a measurably negative effect on quality if they slow the feedback process (CI). You other recommended uses for PRs are also better fulfilled by other ways of organising code review, pair programming and continuous-review (described in this video). Finally on your final point PRs as a teaching mechanism do have a place, but are very poor second compared to pair programming.

@lukashei1870 3 місяці тому

My Question about this is what do you do with unfinished features tho? Releasing unused code into production is a pattern I thought was considered bad?

@ContinuousDelivery 3 місяці тому

Depends who you ask I suppose, and what you mean by "unfinished". There are ways of working that make this safer than the alternatives, this is how SpaceX work for example. The key is that unfinished does NOT mean un-tested. All the code released will be tested and working as designed, though it may not yet, add up in some parts to a finished feature.

@TheLucanicLord 3 місяці тому

"pAre prOgramIng? PayINg Two DaveLaPPar's tO dO oNe job!" I can hear the bean-counters from a mile away.

@ContinuousDelivery 3 місяці тому

Sure, but as is often the case when it comes to Pair Programming, the bean counters are completely wrong. The data from studies of pair programming I think underplays its value, but even so, it consistently says that 2 devs completing the same task as 1 complete it in 60% of the time, but also with MUCH high quality, so overall, if you include the time it takes to fix the bugs that you put into the code Pair Programming is the significantly more efficient approach. I think it is more than that though, because as well as speed and quality, you don't need to spend extra time on code reviews, handovers, and in my experience pair programming is the most effective way to grow the capabilities of teams, whatever their mix of skills. I describe all this and more in this video ukposts.info/have/v-deo/mXmlioCnqYKKzJs.html

@Nephtys1 11 місяців тому

Overall there is a deeper survivorship-bias in the underlying statistical analysis of fewer "blocking" quality gates (like PR) leading to higher quality / delivery performance. Accelerate does go a little into this when analyzing the DORA data, but the focus is on other parts. Still, it should be understood that highly efficient / capable teams should not get blocked by such gates. But there are also less capable teams, or inexperienced ones. Or ones that only start to form skills and trust. In those case removing most gates is dangerous. But we do not get to see any data from such teams, because the company will either hire consultants to fix the damage or go out of business. Therefore we do not see any data from those failing teams in the data. To deal with this bias it is better to start with great teams and get rid of the training wheels one at a time. Given enough time, skills and trust within the team will increase enough for them to gauge quality their own way. That is when to switch to Trunk-based development, not before. The extreme worst case in comparison: a majority of the team is both new and unlikely to stay for more than a few months. Do you really want to switch to asynchronous quality checking of their code (which the video is basically about) if they're unlikely to care for any bad consequences or damage to their reputation? And if you actually weigh by teams instead of projects, this kinds of fundamentally broken teams are just way more likely to happen because highly efficient teams are so efficient they can do a lot more projects within the same time. So if projects follow a normal distribution overall in the industry, there must be more bad teams than good ones because bad teams not only have worse projects, but also deliver fewer successfully. So please do be wary of this bias overall.

@ThomasOwens 10 місяців тому

A couple of things stand out: It seems like you're referring to human-gated pull requests. I'm not sure that pull requests need to involve a human. You could have a series of fast automated checks and, upon passing those checks, merge the code. Some examples of checks would be verifying no linter configuration changes, running your linter/formatter, running static analysis and having no new (major) potential vulnerabilities, running automated tests and passing, no newly disabled automated tests, changes introduced have line coverage. This could also give you a good list of things to review. If your tooling allows you to find merged pull requests that have no human approvals, you can review the change. There's always a chance that future changes could change the context of the pull request, though. Minimally, it points you to the work done that hasn't been looked at by a human.

@ContinuousDelivery 10 місяців тому

I build all of the things that you mention into the commit stage of my deployment pipelines, this is CI not really a PR, and I'd argue that PRs don't really add anything at that point. Why bother creating a branch, when you already have one in the form of 'my-local/main' and you can trigger all the evaluations that you mention on push to 'origin/main'?

@ThomasOwens 10 місяців тому

@@ContinuousDelivery I suppose if you have tools that allow you to use hooks, you can run the checks and rollback the merge. Not all repository hosting services allow for server-side hooks, though. It just feels cleaner and safer to have a remote server with controlled configurations run these checks to avoid tampering. Someone independent from the development team can control the configuration of the tooling used to execute and approve the automated PR checks to ensure that the checks are in place and can't be overridden by the developer of a change. It lets code get into the mainline faster. I'd rather make sure that all the fast required checks pass before integrating and defer some of the more intensive checks to post-integration (like a more robust suite of automated tests, dynamic application scanning, and so on). That isn't to say that I wouldn't rather do pair/ensemble programming and have a robust CD pipeline. This could be more in risk mitigations for cases where you need more human monitoring or involvement in the process.

@ContinuousDelivery 10 місяців тому

You don't really need VCS support for this, CI systems will do the job too.

@mianaviatte 11 місяців тому

I just need to say that obvious thing: LOVE YOUR T-SHIRT!!! It's awesome

@leonardomangano6861 11 місяців тому

It will be awesome to know your opinion about what I think is the biggest issue happening to programming nowadays. What do you think about the common practice of letting Business Analysts and Stakeholders design software? In every project I worked (except for the small ones) Business Analysts and Stakeholders design the application till the most tiny detail and that has a lot of impact on how you implement that solution. A small detail in the UI can impact even your DB Model, in the end people without any technical knowledge is designing the architecture of your system. Is very common to see UX designers closer to the Business team rather than the development team, and that is awful. Also this practice is taking all the fun out of being a software engineer.

@ApprendreSansNecessite 11 місяців тому

Can you describe a bit more that UX designer situation? How are they closer to the business and what are the consequences?

@leonardomangano6861 11 місяців тому

@@ApprendreSansNecessite In my case, UX/UI designers present the mockups to the business before presenting them to the dev team, and there is a lot of bureaucracy required to ask for a change

@ApprendreSansNecessite 11 місяців тому

@@leonardomangano6861 how can the business force you to couple the database and the UI? What lead to this? I am not questioning your testimonial, it is just so odd to me.

@leonardomangano6861 11 місяців тому

@@ApprendreSansNecessite It's not that the business forced me to couple them. The UI and the database model are coupled by definition. The design choices you make in the UI will impact your DB model

@ApprendreSansNecessite 11 місяців тому

@@leonardomangano6861 UIs and databases change at different rates and for different reasons. You cannot blame the business or the UX designer for having failed to put an architectural boundary there.

@stephenreaves3205 10 місяців тому

So this model doesnt allow for bugs because we have tests, but if I write the code and the tests, then any bugs im not aware of will be in both. We make sure reviews are done by separate people but the tests (that we rely on) aren't held to the same standard?

@ContinuousDelivery 10 місяців тому

No, there will still be bugs, there is data to say that there will be about 60% few bugs in production if you adopt a good testing approach, there is no data that I am aware of that indicates that PRs reduce bug count, and in fact when compared to more flow based practices, like CI, then the data is very clear, and quoted in the video, based on the DORA metrics of Stability & Throughput, teams that practice CI score a lot better on both stability (a measure of quality) and throughput (a measure of efficiency). If you practice TDD, which is my preference, you are less likely to have bugs in tests, because the process includes a validation of the test - we always run the test first, before we have created the code to make the test pass, so that we can see that the test is actually failing as we expect.

@bfg5244 11 місяців тому

I've tried Ship-Show-Ask in one project and it worked quite well with the experienced team.

@wal0x 11 місяців тому

With "Ship", there are no code reviews at all, is that right? Cause then, in this aspect that could be even more "radical" than what is described in the video (where some kind of code review always happens at some point if I understood correctly), depending on what kind of changes the team decides can be "Shipped" and what should be "Asked"/"Shown".

@AttilaButurla 11 місяців тому

Yes! We don’t need to follow the same model used on large open source projects.

@cronnosli 9 місяців тому

But need at least to understand that the PR model comes to solve exactly the problems that Trunk base development brings to the table.

@MrCalyho 11 місяців тому

We do this but with a set of different rules. All code goes on to trunk reviewed. Changes should be small, not nessescarily a whole feature. The code that goes in is releasable meaining the release artifact works. Only thing that blocks the release should be code design changes and code that doesn't work. Everything else can be fixed later. This approach works quite well. I know pair programming is better but the human side of pair programming makes it too unpredictable for me. Probably if you do it disciplined enough it would work but I just can't concentrate if someone is looking over my shoulder. I guess if you have a good process like ping-pong it will help it but it takes too much training for the other guy to stick to it.

@SteinGauslaaStrindhaug 11 місяців тому

12:31 Wait, what? Don't most people regularly merge in the main branch (whatever it's called) into their feature branch, like I do?

@ddanielsandberg 11 місяців тому

I'll paste my previous answer... for the 27316th time: "Because it doesn't work. Let's say you have 20 developers, and that each have their own feature branch. Even if all of them rebases from master every hour - how many changes are integrated to the other branches? None! They will stay continuously isolated and you won't know which branches that conflicts with which other branches until one of the conflicting branches is merged into main. CI/TBD is about exposing conflicts early by continuously integrating into mainline. It's nothing strange."

@alexrusin 2 місяці тому

Pair programming and TDD are being rejected for some reason. I wonder why? Maybe because I don't have a bunch of Kent Becks working for me. If I had, I wouldn't need pull requests and code reviews either.

@ContinuousDelivery 2 місяці тому

Maybe, but another way of looking at it, is how do we create the next generation of Kent Becks to work on our team, I'd say through Pair Programming and TDD.

@alexrusin 2 місяці тому

@@ContinuousDelivery I can't agree more. Extreme Programming Explained is a great book. However, the programmers I work with don't have skills to pull off TDD. And if you put two juniors to pair program, you may end up getting the double spaghetti code 🙂 Just yesterday was reviewing code. Test passed, however the way the code structured there was a potential for concurrency issue. Had we released it in production we would have errors happening once in a blue moon and we wouldn't know why. Reality of business and scarce resources forces you to work with what you have.

@slipoch6635 11 місяців тому

How would you go about removing changes that do not work without affecting other changes when not using a feature branch? (assuming there are commits after the bad code has been submitted)

@ddanielsandberg 11 місяців тому

Changes must always work. Incomplete features can be hidden in a bunch of different ways; from build profiles to deployment or runtime configuration, to customer opt-in "try the new layout" options.

@ContinuousDelivery 10 місяців тому

You don't really face this problem, at least not in this form, in real CI. I am generally committing every few minutes, if my code doesn't work, it fails CI then it is only a few minutes work that has failed, so the cause of the failure is usually trivial and obvious, and if it is not, reverting a few minutes worth of work certainly is trivial and obvious. CI is about organising our work so that things are working all the time.

@slipoch6635 10 місяців тому

@@ddanielsandberg and if you have a large set of features that needs removal AFTER deploy? for example a client has asked for some feature, we have implemented it then it needs to be removed cause it is a breach of internal policy (for the client), or is not what they were actually after? (Yeah I know the spec for a feature should be done and checked, but we have had the client change their mind after the fact), now you either have a whole feature that is just hidden with a bunch of extra code, which is not great, or you have to remove it, removing the branch merge is easy enough and should affect later commits by other people, but when it is part of trunk it can be...a git.

@slipoch6635 10 місяців тому

@@ContinuousDelivery SO if you have a large feature where you need to 'break' how the db works for example, you would do all the work to get it up and running using the new system (lets say you are moving from EF to NoSQL because you don't need sharding and all the other overheads MSSQL imposes), or would you implement the first steps, then commit, and work on all the object bases and interface calls to update them to use the new system and commit as you go leaving the overall trunk in a broken or semi-broken state? In the above example someone may be simultaneously working on some controller additions to get a viewmodel using some calculated data alongside some DB model data, in the case of small commits & CI, their work will now not function until the particular model they are pulling in to the controller works and the DB calls/functionality are updated. In the case of the above with the changes made between just EF & EF Core there can be a LOT of hidden gotchas (like whole db methods no longer working the same way) so it can be time expensive to go around hunting all the places it may fail. So person B (controller work) now has what could have been a quick change being delayed, but only if person A's changes are being pulled into their code.

@Zutraxi 4 місяці тому

I will never allow unreviewed code into the open. The moment you are dealing with anything important it has to be seen. If you write small commits for big things sometimes you lose context. I see why ci cd dislike feature branches but this system seem like it introduces entropy and buried errors.

@br3nto 11 місяців тому

14:39 hold on… the branches should be rebased, and so have all the latest changes…

@ddanielsandberg 11 місяців тому

I'll paste my previous answer... for the 27315th time: "Because it doesn't work. Let's say you have 20 developers, and that each have their own feature branch. Even if all of them rebases from master every hour - how many changes are integrated to the other branches? None! They will stay continuously isolated and you won't know which branches that conflicts with which other branches until one of the conflicting branches is merged into main. CI/TBD is about exposing conflicts early by continuously integrating into mainline. It's nothing strange. What is strange is the obsession with branches to isolate changes and hide systemic issues."

@DevinWeaver 11 місяців тому

Thanks!

@slavagarshin 11 місяців тому

Easy-peasy. The solution to waiting for feature review is not review the feature, but split feature into small tasks which could be reviewed in minutes. I do. No "ready for review" task state, just a message to the "Mearge Request" chat is required. If I'm not on the meeting then response will be in 5 minutes. If I'm on meeting and not currently speaking than the same. Another point that I count releasable any code that compliled and deployed successfuly. Trunk base development with feature switching is expensive, we just open a button or api when the whole feature is ready.

@evgeniikhandygo8282 11 місяців тому

Another problem I can see is people. Some may just feel uncomfortable knowing their code can go to production without a code review. The process allows to account for that by doing on demand pair programming of course, but this just goes to show that it is not suitable for everyone. The team has to be fairly mature I think.

@ryanmorris5709 11 місяців тому

But… how does CICD work when you have changing DTO’s between multiple applications. Don’t you need major version upgrades which means you can’t have everything on one branch?

@ruggerovisintin4687 11 місяців тому

When you have a distributed system, you have to design a breaking change so to be back-comptabile or be rolled back until every other service is being updated (if it's only stuff internal to your team) or until a certain period has passed (publicly exposed stuff). In either cases you mark the old use as deprecated to make sure it's not used anymore. That way you ensure business continuity and ensure roll-back ability in case things might go wrong (it can and will happen). Same thing for migrations

@barneylaurance1865 11 місяців тому

Usually you can add a field to a DTO that your application sends without breaking any consumers. If you want to delete a field, ask your consumers to stop using it, let them stop using it, and then later remove it. In a big system you might have to phase it out more gradually and tentatively.

@johnridout6540 11 місяців тому

When you have multiple applications that communicate you should have an explicit API. Each application is its own project that works for specific versions of the API. The API may even have its own repo.

@SolidousMdz 11 місяців тому

Sorry but I missed the link for the article mentioned in the video.

@wal0x 11 місяців тому

There is a comment by JinKee where Dave posted the link in response.

@orange-vlcybpd2 11 місяців тому

Also trunk development would make the team members start to own the whole codebase and not only their own branch. But yeah, that requires maturity from the members. But maturity comes with responsibility. So, kind of a chicken-egg problem :) Regarding the situation where you need to roll back changes for the feature, and imagine, you have several team members contribute their parts to the feature, like backend, frontend and e2e tests, you may get into troubles, having to search for every single commit related to the feature, and removing it. And if those commits contained any unrelated changes, this may break the product. To overcome this, you then also need to have a rule like, that feature releated commits are clearly separated from any other code improvements you want to make to the code. Well, again, that requires high maturity and sense of responsiblity from the team members. But yeah, i can confirm, the merging hell with feature branches is real. Another scenario, how do you make breaking experimental changes with trunk based development? I see no other solution than experimental branch, but that would be much less frequent event, than merging hell on a regular basis.

@zoladkow 9 місяців тому

yeah, you're actually bringing up a real life case vs. the ideal ones that Dave used to showcase his ideas. I mean, on a living system, when you need to change a feature (and if it is also a dependency for others) what do you do? I think i will anserw myself, that one needs to actually create a new feature, not change the old one, then use feature flags to switch one for the other when ready. still, this is most often hard to sell as everyone sees such aporoach as more effort 🤷

@zillyoldfart 11 місяців тому

Am I missing something? Changes go into the review state. Changes go to integration after a review by someone else - how is that not a gate or a delay? Is the review optional or done after the integration?

@wal0x 11 місяців тому

From the article: "The fact that non-reviewed code can be tested or deployed in production is surprisingly the most significant benefit. We do not ever block the flow of work through the value stream"

@zillyoldfart 11 місяців тому

@@wal0xThanks - I think that this key issue (the non-reviewed code) is not emphasized enough in the video presentation. Perhaps my bad.

@alexanderstohr4198 10 місяців тому

Doing something new... is often a topic of courage to take a first step for trying it out.

@davidboreham 10 місяців тому

$0.02/wo: The PR seems to have come from organizations that have a particular management style and culture (Google?, and perhaps Linus) where the idea is that "you are not worthy" to commit code to "my repo". So you are expected to doff your cap, bow, and present your miserable code for blessing by the priests. I'm sure this works fine in that culture. But most of us don't have that culture. We hire developers that we expect to be competent. We fire them if they're not competent. So the whole notion of bowing and scraping to be allowed to commit code is not applicable. Instead we might want to have a manager or dev lead _look_ at code being committed in order to catch bad code and maintain their list of people to fire or send for additional training, but there's no need for a review-delay-gate. We might also want to have a formal code review process in some circumstances, or for some kinds of code (e.g. security-related). I think this is roughly what Dave is describing in this video. The PR isn't designed to facilitate that, but there is a need for some tooling support -- a kind of "code queued for review" feature in the sccs -- otherwise how would we know what code had changed in the development of a particular feature, after the fact?

@ContinuousDelivery 10 місяців тому

I agree with the core of your point, perhaps not the cause, I think PRs come from the open source world, where it is sensible to be a bit wary of unknown people committing to the core of a widely-used operating system (for example).

@davidboreham 10 місяців тому

@@ContinuousDelivery Fair point, but I think also used as an excuse to justify assish behavior.

@Mauro-K 10 місяців тому

this sound great, I can already see the face of disagreement from the "white collar" part of the team

@moltar2000 11 місяців тому

Limitations of pairing: - time zones (distributed team) - language barrier (team mates don’t share the same primary language) - cultural differences, many cultures have a completely different work ethic and culture.

@ApprendreSansNecessite 11 місяців тому

It looks like you are listing limitations for communication, not pairing.

@Blob64bit 11 місяців тому

Exactly. Sounds like the communication is the underlying issue here.

@AndrewEddie 11 місяців тому

+ Budget (aka, not allowed to hire) is also a limitation. + Overcoming the perception that one person in the pair is "not working"

@johnridout6540 11 місяців тому

@@ApprendreSansNecessite Communication is a limitation for pairing

@ForgottenKnight1 Місяць тому

In that case, you can do pair programming 2,3 hours a day (you're lieing to yourself if you say you'll do it 8 hours a day) Language barrier - this seems to me like a communication issue, so improve that over time. Cultural differences - same as language.

@br3nto 11 місяців тому

13:35 wait what? Local unit tests and integration tests should give you a big clue if your changes are good or not

@ddanielsandberg 11 місяців тому

But most developers living in a FB/PR-driven environment does not have the capability to do that (due to bad architecture, build system, wrong technologies). They replace the tasks in build scripts with steps in a pipeline and the only way "to know if their changes works" is to make a feature branch and push to it.

@stephenreaves3205 10 місяців тому

I hate when people push pair programming. My small team is spread between west coast US, east coast US, and Eastern Europe. How the heck am I supposed to shoehorn that in?

@ContinuousDelivery 10 місяців тому

I have done this, pair in the periods when the timezone overlaps.

@mohitmalhotra9034 10 місяців тому

Skip to 16:30 to save your time. That's where the alternative approach starts

@ContinuousDelivery 10 місяців тому

... and then you miss the real lesson 😉😁

@rexgamez 25 днів тому

So how mob programming can be faster than PR's, when at least 2 people are working on one task (which in reality means than one is writing code and others are more or less actively watching it) instead of 2 people working on 2 separate tasks and then review each others work?

@ContinuousDelivery 24 дні тому

Because it is the thinking that takes the time, not typing the tasks, and more people can sometimes come to an answer faster than an individual working alone. I don't know of any research into the efficiency of mobbing, but there is lots on pair programming, and the data there is pretty consistent. A pair of people will finish a task in 60% of the time of an individual working alone. Not quite twice as fast, be nearly, but the pari produce significantly higher-quality output, so when you include the time to diagnose and fix defects, the pair is considerably more efficient. I describe this here: ukposts.info/have/v-deo/mXmlioCnqYKKzJs.html

@MisFakapek 11 місяців тому

strongest issue with this assesment is that developers are more often than not: inexperienced, unskilled, messy, chaotic, conceited and arrogant. It's not that it's intentional but developers' ego are something that is making things like CI/CD is effectively impossible.

@robertkelleher1850 10 місяців тому

Asynchronous Pair Programming!

@unexpectedkAs 7 місяців тому

This video asusmes that the developers in your tram are good. Around 80% of devs i worked with for 20 years have been mediocre at best. Try doing a medical device without code reviews / PRs. Of course this approach can be improved, but after reading the full trunk baded development psge it was clear to me thst it is not for sny of my teams. Love tour videos by the way!

@ray30k 11 місяців тому

Loving the subtle homestuck shirt

@chakala2149 11 місяців тому

I agree Dave, but our team sucks too much to practice real CI

@ContinuousDelivery 11 місяців тому

Wait for the next video!… 😉

@FudgeYeahLinusLAN 11 місяців тому

@@ContinuousDelivery I can't wait for this video, because my team sucks too.

@JinKee 11 місяців тому

Who is Thierry and where can I learn more?

@ContinuousDelivery 11 місяців тому

There is a link in the description to this video, but also here: bit.ly/3JiJUxV

@wal0x 11 місяців тому

Maybe I'm too tired already, but I scrolled through the video description several times and couldn't find the link.

@mil3761 11 місяців тому

I love your t-shirt and what it says 😂

@bodungus3666 10 місяців тому

I worked at a large corp who's main reviewer unironically would block PR's because fields weren't in alphabetical order. It was fucking hilarious!

@vyli1 3 місяці тому

Instead of claiming that data shows that pull requests affect quality negatively, how about actually backing up that claim with actual evidence? It is interesting, because Google is also utilizing something like pull requests and they have made internal studies into the effectivness of code reviews and many aspects of code reviews and their internal studies seem to suggest that it absolutely makes sense (as stated in the book 'Software Engineering at Google'). How come that Google came up with a completely different conclusion than whatever data you have?

@ContinuousDelivery 3 місяці тому

Well, There are links in the notes on this video to the data that backs these claims and I don't know which data from Google that you are referring to, Google is a big org, but my primary source is the data from the DORA metrics, which is now owned and managed by Google! This is based on the most scientifically valid study of software development practice that I know of, and on the results of 10's of thousands of survey responses.You can read about all this and the reason to trust this data more than anecdote, in the Accelerate book.

@vyli1 2 місяці тому

@@ContinuousDelivery I have looked through the links in the description, none of the links provides any data about pull request reviews. Which link do you mean? As for DORA data, that's again very vague. Which DORA data point to pair programming better than code reviews? Under what settings? I don't know where to look for the data that you claim you have. I wasn't able to find any data of pair programming vs pull request on the internet, where were you able to find that data?