7 Comments

Will gonna implements this 🙌. As i am in starting phase of my career, code review and quality matters a lot to me. I always try to write easy to understand and working code. And also focus on code review that helps me to improve and increase my growth.

Expand full comment

How do you balance the test batching with testing individual PRs enough to feel confident they will be approved?

My immediate thought was having good guidelines around unit tests vs integration tests. I can also see it helping to push for smaller PRs since it’s easier to have high confidence in smaller changes. It seems like you need your SDLC set up to support batch testing though since I wouldn’t want to batch 3 PRs that have 1000+ lines changed.

The batch testing could also cause problems since it might not be clear which of the changes is causing the issue. How do you deal with that?

Expand full comment

About batching, I typically only batch testing for changes that are loosely related on the same code path.

If my changes were affecting different code paths then batching testing wouldn't make sense given running the code wouldn't give me enough coverage to feel confident.

So as usual, it depends on the changes you're planning and how you want to manage risk.

Expand full comment

About hinting with diff titles - that depends on a person who's doing the code review. What looks easy for you can be not that easy/obvious for reviewer. I agree with descriptive PRs though.

Feature flags are great, but they may not cover all the cases. You can hide your features behind a feature flag in mobile app, but backend doesn't necessarily do that with their DB migrations (while technically possible, I haven't seen it)

Expand full comment

If it’s tiny (e.g a simple one line change), it’s easy in most cases

I don’t think gating in the back end is as uncommon as you think

Expand full comment

how do you write small diffs without using feature flags? sometimes it seems heavy handed to add a flag just so you can land to main. but if you don't use a flag, it might not get approved for fear of a partially incomplete feature being merged in. the other option I see is to create a bunch of diffs that stack onto each other, so merge diff1 into main, create a PR for diff2 merging into diff1, one for diff3 merging into diff2, etc

Expand full comment

Usually stacked diffs. This lets me break up features into stacks of commits that can individually be reviewed and tested

Expand full comment