-
Notifications
You must be signed in to change notification settings - Fork 2
Tips on how to Code Reviews
Someone asked you to review their PR! But how should you go about it? What should you be looking for?
There should be a link to the ticket and a few bullet points highlighting what functionality the PR changes are intending to implement. Before you even start looking at the code, ensure you understand the requirements of what the code is trying to accomplish.
Now that you've reviewed the requirements of the ticket, at the bare minimum, do the code changes match the requirements?
Are they following best practices? For styling, is it in a stylesheet instead of styled in-line? For API calls, are they done in a service instead of scattered everywhere?
Does it really require more than two for loops? Would a data structure help? If it seems like a lot of work is being done to achieve something simple, suggesting a more efficient solution is needed.
If there's ambiguously named variables or functions, inconsistent formatting, the PR may be more difficult for you to read. And if its difficult for you in terms of readability, chances are it will be difficult for future developers to read that code section. A request for improving readability may be needed. Requesting for in-line comments or function descriptions may help too!
It's hard reading someone else's code, when sometimes you barely understand your own haha. When in doubt, don't be afraid to ask the PR creator questions to clarify, or explain something that you may not be familiar with!
If you're impressed, say so! If they did something well you wouldn't have thought of, comment so! 😆
At the end of the day, its the assignee's responsibility to ensure all the requirements are met and not the code reviewer's. There's an appropriate balance between pointing out potential issues and providing solutions. Let's encourage each other and help each other improve (: