Spot on, Ryan! Long diffs have a higher chance of a random LGTM occurring than smaller diffs! Because I self-review my PRs before requesting reviews, I'm encouraged to make those smaller diffs anyway. 😃
What do you do when you have dependent changes? I'll reuse your example:
> imagine I have a simple server <> client setup and I want to log info from the client in the backend. Here's how this might look:
> Diff #1 - pass field from client in request to the server
> Diff #2 - add the new field to the logging event schema from the backend
> Diff #3 - read the field from the client request and log it using the new event schema
In this example, suppose you don't know the exact format you need in the server until after writing diff 3. Would you code the entire flow and then break it into 3 stacked diffs?
> Would you code the entire flow and then break it into 3 stacked diffs?
That’s correct. I’ll get it all working together on my local in a case like this. Then I’ll selectively add changes to each commit for the reviewers sake
I agree, small diffs are better than larger ones, but with smaller diffs you could lose context of the much larger change. How does a dev know the full picture.
Stacked diffs offer the opportunity for larger changes to be reviewed as well as smaller ones, so consider utilising this for a more well rounded approach to staying small, but keeping context 😄🙌
I'd recommend that each diff do just one thing. For example, imagine I have a simple server <> client setup and I want to log info from the client in the backend. Here's how this might look:
Diff #1 - pass field from client in request to the server
Diff #2 - add the new field to the logging event schema from the backend
Diff #3 - read the field from the client request and log it using the new event schema
We could've done (2) and (3) in the same diff, but it's cleaner if we separate them out.
Might be exaggerating, but I’ve always said that the best diff/PR size is the one that fits the smartphone screen or possible to review on the go :)
Spot on, Ryan! Long diffs have a higher chance of a random LGTM occurring than smaller diffs! Because I self-review my PRs before requesting reviews, I'm encouraged to make those smaller diffs anyway. 😃
> I self-review my PRs before requesting reviews
This is great Akos, every engineer should be self-reviewing their PRs before sending them out
Great article Ryan! This exact topic is on my list to cover as well. An added bonus is small CLs can be used to pad your ~coding stats~ :P
Hahah true, love seeing the count go up
What do you do when you have dependent changes? I'll reuse your example:
> imagine I have a simple server <> client setup and I want to log info from the client in the backend. Here's how this might look:
> Diff #1 - pass field from client in request to the server
> Diff #2 - add the new field to the logging event schema from the backend
> Diff #3 - read the field from the client request and log it using the new event schema
In this example, suppose you don't know the exact format you need in the server until after writing diff 3. Would you code the entire flow and then break it into 3 stacked diffs?
> Would you code the entire flow and then break it into 3 stacked diffs?
That’s correct. I’ll get it all working together on my local in a case like this. Then I’ll selectively add changes to each commit for the reviewers sake
How to break changes, if it is features development having lot of change to merger master through CI/Cd
I agree, small diffs are better than larger ones, but with smaller diffs you could lose context of the much larger change. How does a dev know the full picture.
Stacked diffs offer the opportunity for larger changes to be reviewed as well as smaller ones, so consider utilising this for a more well rounded approach to staying small, but keeping context 😄🙌
Good luck with branch management and rebasing!
Yep, love stacked diffs. Great for breaking down features into sets of separate diffs
Any suggestions on how to manage it? For example - should diff contain complete feature/bug fix or can be partial?
I'd recommend that each diff do just one thing. For example, imagine I have a simple server <> client setup and I want to log info from the client in the backend. Here's how this might look:
Diff #1 - pass field from client in request to the server
Diff #2 - add the new field to the logging event schema from the backend
Diff #3 - read the field from the client request and log it using the new event schema
We could've done (2) and (3) in the same diff, but it's cleaner if we separate them out.