This Is How REAL Senior Developers Review PRs | PR review hacks | Code Decode

Summary

This video provides practical guidance on conducting efficient and constructive PR reviews in software development. Key topics include the importance of consistent coding standards, mentoring team members instead of criticizing, and balancing speed with code quality. It emphasizes the responsibility of PR reviewers to maintain the integrity of the codebase by flagging critical issues while also fostering team growth. The video also highlights best practices for handling tech debt, preventing bad code from reaching production, and improving collaboration within the team.

Takeaways

• 😀 Consistency is key in code reviews. Ensure the team follows uniform naming conventions and coding styles to enhance readability and collaboration.
• 😀 PR reviewers should focus on constructive feedback rather than accusations. Asking 'why' helps developers reflect on their choices and improve their approach.
• 😀 Don't overload PRs with excessive praise or negative comments. Focus on actionable feedback to improve code quality.
• 😀 Block code only when there are critical issues like incorrect logic, security risks, or untestable code that could jeopardize production.
• 😀 Avoid 'perfect being the enemy of good.' While speed is important, never let bad code slip into production to meet deadlines.
• 😀 PR reviewers are not just gatekeepers but mentors. Their role is to guide the team, ensuring the code is clean and aligned with best practices.
• 😀 Minor issues, like improper annotation styles, should be addressed in comments for future PRs, rather than blocking the current merge.
• 😀 When facing non-blocking issues like inconsistent logging, create a tech debt ticket to address these improvements in future sprints.
• 😀 Encourage collaboration by offering solutions, not just identifying problems. Be a resource for the team, not just a critic.
• 😀 Celebrate the good aspects of code in reviews, but avoid mentioning them in the PR comments to prevent overload for other developers.
• 😀 Reviewers should always think about the long-term health of the codebase. Document insights from code reviews to prevent repetitive mistakes in the future.

Q & A

Why is consistency important in PR reviews?

Consistency in coding standards, such as naming conventions and annotation styles, helps improve code readability and maintainability. When the team follows the same practices, developers can focus on solving problems instead of spending time on figuring out the code's structure.

What should be done when you encounter performance issues like nested loops in a PR?

If you spot performance issues, such as nested loops that increase the time complexity of the code, you should suggest more efficient alternatives, like using a map to reduce complexity. This helps optimize the code and improve performance.

How should PR reviewers approach providing feedback?

PR reviewers should adopt a respectful and constructive approach. Instead of accusing the developer of mistakes, they should ask questions like 'Why did you do it this way?' and suggest solutions to improve the code without demotivating the developer.

When should a PR be blocked during the review process?

A PR should be blocked if it contains critical issues, such as incorrect logic, security vulnerabilities, or non-testable code. These issues need to be fixed before the code can be merged into the master branch.

What are the risks of not using caching in certain scenarios, as discussed in the script?

Not using caching when handling large datasets can cause performance problems. For example, if you query a large dataset and transfer all the data to the front end, it could lead to high CPU utilization and delays due to network latency.

What is the role of a PR reviewer beyond just identifying issues in the code?

A PR reviewer is not just a code enforcer; they are a mentor and a defender of the code quality. They should guide the developer, suggest improvements, and celebrate good practices to foster an environment of learning and growth.

What is a Tech Debt ticket and when should it be created?

A Tech Debt ticket is created for non-blocking issues that do not immediately affect the functionality of the application, such as inconsistent logging formats. These issues should be addressed in future sprints but do not need to block the PR from being merged.

How can PR reviews help improve long-term team collaboration?

PR reviews, when conducted effectively, help build trust within the team and encourage knowledge sharing. By documenting key learnings from each review, teams can avoid making the same mistakes repeatedly and foster better collaboration and code quality over time.

What should be done when a developer submits a PR with urgent changes that affect production?

Even when there are urgent changes, you should not rush the review process and let bad code slip through for the sake of speed. Quality should not be compromised for velocity. Ensure the code is thoroughly reviewed before merging into the production branch.

Why should good parts of a PR be highlighted during the review?

Highlighting the good parts of a PR helps to motivate the developer and build a positive relationship between the reviewer and the team. It balances the feedback, showing that you appreciate the effort while also pointing out areas for improvement.