Pull Request Reviews
What you should look for when reviewing a pull request. Can also serve for you to understand good coding practices.
Reviewing Pull Requests
The main goal of a review should be to verify the following, and to provide constructive feedback:
The code is documented well.
The code is well written.
The code is styled properly.
The pull request itself is properly formatted – refer to the
Git
section of the bible.
Pull requests should be only accepted if all categories are satisfied (with an additional judgment call for any other comments), and requesting changes should be done liberally – rather than accepting and asking to make changes before merging.
Documenting code
Variables should be well named. Consider the following two snippets:
The same should be done for functions:
In the case that the purpose of the function is not easily understood from the name, it is required to provide a comment before the function header describing the function's purpose and what the return value represents.
Comments should be used liberally, especially for complex or unusual snippets:
There is no reason to have code that is difficult to understand.
Well-written code
Whether or not code is well-written is up to the discretion of the reviewer, but there are certain guidelines to keep in mind. (The following snippets are in python but convey the same points)
Avoid Unnecessary Computations:
Avoid Excessive Variable Declarations
Avoid Excessive Commenting
Code Styling
We will be adding more java specific code style guidelines in the future, but here are some basic thing to keep in mind:
Use camelCase for multi word variables
Proper use of spacing
Every function should be separated with exactly one empty line and there should not be unnecessary breaks within functions.
Lines should be at a readable length and it is up to the reviewer to make a final decision. Suggested is to keep lines to shorter than 100 characters.
Last updated