Smithery is now a part of Arcade.dev! Read more in our announcement here
Provides practical guidance for conducting thorough code reviews that identify issues early, promote knowledge sharing, and deliver constructive feedback...
This skill provides guidance for conducting effective, constructive code reviews that improve code quality and team collaboration.
Goals of code review:
Not goals:
Does the code work as intended?
Does it fit the system?
Can others understand it?
Is it adequately tested?
Are there security concerns?
Will it perform well?
Before reviewing code:
High-level first:
Then details:
Good feedback:
Example:
Consider using a Set instead of an array here for O(1) lookups.
With the current implementation, the nested loop creates O(nĀ²)
complexity which could be problematic with large datasets.
Types of comments:
Blocking (must fix):
Non-blocking (suggestions):
Positive:
Problems (objective):
Preferences (subjective):
Rule: Block on problems, discuss preferences.
Instead of:
Say:
Weak comment:
This should be refactored.
Strong comment:
Consider extracting this logic into a separate method.
It's used in three places and would be easier to test
and maintain as a standalone function.
Use questions to understand and guide:
Don't only point out problems:
You might be wrong:
Long methods/functions:
Duplicated code:
Complex conditionals:
Large classes:
Magic numbers/strings:
Define standards for:
Document standards:
Use a consistent checklist:
If it's a preference:
If it's a problem:
Listen and understand:
Discuss, don't dictate:
Aim for:
If you can't review promptly:
Optimal PR size:
For large PRs:
Use reviews to share knowledge:
Help others become better reviewers:
ā Nitpicking without value - Focus on meaningful improvements ā Blocking on style preferences - Use automated tools instead ā Rewriting in your style - Respect different approaches ā Reviewing too quickly - Take time to understand ā Being overly critical - Balance criticism with praise ā Ignoring context - Consider constraints and tradeoffs ā Making it personal - Focus on code, not the person
Code meets standards and is ready to merge:
Minor issues that don't block merge:
Issues that must be addressed:
Fundamental problems requiring redesign:
Good review comment structure:
Example:
[Blocking] This query will cause N+1 problem when loading
related records. Consider using eager loading with includes()
to fetch all data in a single query. This will significantly
improve performance with large datasets.
Remember:
