Think about your team workflow. Then try what makes sense for your context. Then adjust. The most exciting words in engineering are, “It turns out…”. A recent Twitter kerfuffle about the pull request team workflow happened to mention me. The last thing I want to do is to participate in another pissing contest about code reviews, but here I am.
For synchronous/non-blocking, one possibility is partial pairing. I've often seen teams pair on what they consider to be significant changes but do solo work on what they consider small things.
Think. Experiment. Share. -> most important part imo.
These arguments usually come from the desire to improve (certainly why I keep starting those arguments a lot). The part that a lot of people miss is, that is is ok to try different things and that it is also always ok to go back to the way you were (while keeping the J curve in mind - some techniques require some practice before you can truly say they didn't work).
Certain techniques have a good likelihood to work. Small change sets are less risky, easier to debug etc. That doesn't mean they are immediately applicable - sometimes other process improvements would have to be made first to enable to true power of small, frequent changes. But if all you ever do is argue over the best way to do X or Y, nothing is ever going to improve. As Jeffrey Fredrick said, if you want to be better tomorrow, you need to be different to how you are today. The only way to do that, is by experimenting.
I think there's a potential other dimension related to proximity. Working within a team on the same codebase is one thing, working across teams or groups can be quite another. The overhead of coordinating reviews and feedback on PRs, especially across teams and groups can be considerable effort.
"I think there is fruitful space to explore in the bottom right.". We currently work on this topic and created a "Practices review" format (asynchronous, non-blocking, with the whole team at once). This idea is to discuss once per week with our team about various topics and best practices identified during the code reviews (or directly while coding). This format is used a a lot of teams in France and the feedbacks are really encouraging. It helps teams to have less comments in code reviews while sharing knowledge more efficiently. I discuss about this format during a webinar recently (with a terrible english accent): https://youtu.be/cqxtE-DcvcA?t=2296
Regarding the large pull request time to merge time. This analysis on open source project leads to a different conclusion. I guess thing are different inside a company.
"Our analysis shows no relationship of pull request size and composition to the time-to-merge, regardless of how we partition the data: day of the week the pull request was created, affiliation to industry, and programming language."
Very interesting article, especially the "dimensions of variability" part. I'm wondering how it connects with https://martinfowler.com/articles/ship-show-ask.html. Would it be "ask" = "blocking/async", "ship" = "nonblocking/async", and "show" = "nonblocking/sync"?
I'm surprised about cataloguing pairing as blocking. The "review process" of pairing happens as you are working, therefore, once the code is done you can push into production, nothing has been blocked, no additional step that needs to be done. I see that as one of the biggest advantages of pair programming. So I would catalogue that as Non-Blocking Sync.
To break the Time/Delay/Size cycle, some places use a “stacked diff” system that lets PRs be submitted for review while subsequent work accumulates to a later PR. This is awkward with GitHub but there are things like this to help with it: https://cord.com/blog/stacked-diffs-on-github-with-spr/
For synchronous/non-blocking, one possibility is partial pairing. I've often seen teams pair on what they consider to be significant changes but do solo work on what they consider small things.
"then you’d be notified in detail & immediately" Definitely curious about this :)
Think. Experiment. Share. -> most important part imo.
These arguments usually come from the desire to improve (certainly why I keep starting those arguments a lot). The part that a lot of people miss is, that is is ok to try different things and that it is also always ok to go back to the way you were (while keeping the J curve in mind - some techniques require some practice before you can truly say they didn't work).
Certain techniques have a good likelihood to work. Small change sets are less risky, easier to debug etc. That doesn't mean they are immediately applicable - sometimes other process improvements would have to be made first to enable to true power of small, frequent changes. But if all you ever do is argue over the best way to do X or Y, nothing is ever going to improve. As Jeffrey Fredrick said, if you want to be better tomorrow, you need to be different to how you are today. The only way to do that, is by experimenting.
I think there's a potential other dimension related to proximity. Working within a team on the same codebase is one thing, working across teams or groups can be quite another. The overhead of coordinating reviews and feedback on PRs, especially across teams and groups can be considerable effort.
"I think there is fruitful space to explore in the bottom right.". We currently work on this topic and created a "Practices review" format (asynchronous, non-blocking, with the whole team at once). This idea is to discuss once per week with our team about various topics and best practices identified during the code reviews (or directly while coding). This format is used a a lot of teams in France and the feedbacks are really encouraging. It helps teams to have less comments in code reviews while sharing knowledge more efficiently. I discuss about this format during a webinar recently (with a terrible english accent): https://youtu.be/cqxtE-DcvcA?t=2296
would TCR test && commit || revert workflow fit in Sync / Non-blocking?
Thanks for the post.
In our team we use "Ship / Show / Ask" strategy and it works pretty well
Regarding the large pull request time to merge time. This analysis on open source project leads to a different conclusion. I guess thing are different inside a company.
"Our analysis shows no relationship of pull request size and composition to the time-to-merge, regardless of how we partition the data: day of the week the pull request was created, affiliation to industry, and programming language."
source: https://arxiv.org/pdf/2203.05045.pdf
Did you start to implement the your newsfeed like review at the end of the post? I'd be interested to hear more about that.
Very interesting article, especially the "dimensions of variability" part. I'm wondering how it connects with https://martinfowler.com/articles/ship-show-ask.html. Would it be "ask" = "blocking/async", "ship" = "nonblocking/async", and "show" = "nonblocking/sync"?
I'm surprised about cataloguing pairing as blocking. The "review process" of pairing happens as you are working, therefore, once the code is done you can push into production, nothing has been blocked, no additional step that needs to be done. I see that as one of the biggest advantages of pair programming. So I would catalogue that as Non-Blocking Sync.
The question mark could be an AI based IDE tool that comments /highlights problems, but unlike a human pair, can be easier ignored.
"I think there is fruitful space to explore in the bottom right." Did you mean to say "the top right"?
Very good observations. I wonder what do you think about how DORA metrics and how trunked based development affects this train of thought.
https://cloud.google.com/architecture/devops/devops-tech-trunk-based-development
To break the Time/Delay/Size cycle, some places use a “stacked diff” system that lets PRs be submitted for review while subsequent work accumulates to a later PR. This is awkward with GitHub but there are things like this to help with it: https://cord.com/blog/stacked-diffs-on-github-with-spr/