How to apply the 'one diff, one thesis' to improve code reviews?

Thanks Jordan!

You're right, i could not find the video either when searching by "One Diff, One Thesis". My bad.

Here is a link to the video: https://www.jointaro.com/lesson/3NAbmgiaY3IpvayNIoVl/one-diff-one-thesis-what-every-code-review-should-be-like/

You got my question right. Thats exactly what i was wondering about.

I was thinking about that exact same strategy you describe so im very glad to hear you mentioning as i was not sure this was the way to go. Will give this a try!

I will also take a look at https://graphite.dev/stacking, looks very interesting.

Thanks again!

How does the 'one diff, one thesis' workflow looks like?

Should i make multiple PRs each one of around 250 lines?

Tactically, there's a lot of tools, and we had a similar post for Github some days back that might help you. And yes, generally each of your PR should be smaller than 250 lines (250 is considered on the large end).

Having said that, in my personal experience, it's quite time-consuming to break apart large PR/Diff/CLs.

The main reason most people end up in such a situation is because they try to implement a working, end-to-end version of the feature before they send anything for review.

While this is understandable - I remembered being scared to send in something I didn't know whether it worked, this is also is counter productive. Don't do this if you can. (If you did, treat this as "prototype code", and rewrite it in a new branch with the following approach.) Instead, it's best if you strategize your implementation such that you don't end up in this situation.

To do this, you need to break down the feature into distinct chunks, each with a "thesis", such as "implement this button". A thesis should have enough function to be verifiable via manual or automated tests. Alex has an amazing, in-depth example on the Taro blog on how do this, I recommend giving it a read.

Once you've properly decomposed your feature into distinct chunks, then figure out what's the one thing everything else depends on. Start with that chunk. Spend time cleaning up your code in that chunk. As soon as you think it's good, send out a PR for that chunk. These PRs should be naturally be fairly small because they aren't trying to do too much (PR for "Make an API call in Android" is probably 100-250 lines code, depending if you have test coverage). Keep working on the next chunks while this is in review. Submit and rebase. Rinse and repeat.

I'm happy for the question, though I do want to share a bit of feedback. When asking the question that involves external context, try to provide the resources or summarization necessary for the answerer to get caught up on that context as quickly as possible.

To roughly quote Alex, "make it easy for the person answering your question to help you"

So with that said, I don't really have context on this "One Diff, One Thesis" video; I tried to look it up but didn't see anything. But I can take a guess at it and give you advice based on what I got in your question.

This is going to be short and simple but, "Make a stack." PRs should really only be > 500-1000 lines if you are doing some large change across the codebase that literally requires it to be that long, like a library upgrade or implementing a new linter rule or migration or something.

A PR stack involves having a feature branch, which then has multiple branches over time branching off of it to implement small parts of that feature. It may be a small piece of the logic for example. As long as each PR gives the broader context to the rest of the PRs and the vision as a whole, you should be fine to take this approach for something you would have considered to be a 2000 line PR otherwise. A tool called Graphite exists that makes making stacks easier https://graphite.dev/stacking for people who don't have the tooling at their company by default (like there is at big tech companies).

I hope this helps!

Awesome! So glad that’s helpful. Thanks for posting the original context. That’s helpful. Have a great night!

Thanks for the references Kuan!

These two thing you mention are very true:

1. it's quite time-consuming to break apart large PR/Diff/CLs.
2. "...trying to implement a working, end-to-end version of the feature before they send anything for review."

I like the approach on your last paragraph!

Or maybe it refers to have a thesis per commit within that PR?

This way, when reviewing on github for example, you can check changes per commit.