DEV Community

Cover image for Don't ask me to review your huge PR
Rickvian Aldi
Rickvian Aldi

Posted on

Don't ask me to review your huge PR

TL;DR Huge PRs are bad. Break it to smaller PR,
”as small as possible, as big as necessary.”

Image description

There are lots of benefits on Smaller PR. I'm giving perspective of problem with huge PR provides better understanding of why it matters:

Problems of Huge PRs:
Complexity:
Difficult to understand the scope and impact of changes.
Increased chance of missing bugs or issues during review.

Review Fatigue
Reviewers may experience fatigue, leading to less thorough reviews.
Critical feedback may be overlooked.

Merge Conflicts
Higher likelihood of merge conflicts, especially in active codebases. Resolving conflicts can be time-consuming and error-prone.

Delayed Feedback
Engineers have other task to do, pushing them to review your huge PR soon will messed with their working schedule, this also means indirectly increase THEIR cycle time.
Longer review times delay the feedback loop.
Increases YOUR cycle time.
Slower integration of features or bug fixes into the main codebase.

Deployment Risks
Larger changes increase the risk of introducing significant issues.
Harder to isolate the root cause of problems if they arise post-deployment.

Reduced Collaboration
Larger PRs can discourage collaborative review processes.
Team members might find it challenging to stay informed about ongoing changes. Navigating history of Pull request will be painful

Disrespect to other engineer
unless you are inexperienced engineer, i will consider it disrespect to rise huge PR

Category
Estimated First-loop Review time needed (dedicating time)

Small PR
< 30 minutes

Medium
Up to 1 Hour

Large

1 hour

Subjective estimation and can change. Line and file changes may not be good proxy.

Rules & Guidelines

Non-urgent changes on app behaviour or refactor must include associated ticket to track history and reason why it done.

Split Large PRs:
Split large PRs into smaller, more manageable chunks.
in case you have bad commit management discipline, nothing stop you from creating multi PR in same ticket
feature/TASK-1234-part-1-scenario-a
feature/TASK-1234-part-2-scenario-b
feature/TASK-1234-part-2-test-and-refactor

But supposedly ticket shouldn’t be huge at first place. If i find i have huge PR, normally i will look at how i breakdown task in ticket and breakdown further.

Context and Documentation: Normally small enough PR already self explanatory. You may provide detailed descriptions and context for the changes to facilitate easier reviews.

Collaborative Review: if you must, for very large or complex PRs, consider collaborative reviews or pair programming sessions. This speed up feedback-loop process and allow everyone to understand further.

Top comments (0)