Code Review Guidelines
This sections contains all the information related to how to get your code reviewed for a Diffgram contribution and also guidelines for reviewing new code to be added to the system.
All pull requests created in the Github repository should go through our review process to make sure that the code works, is effective, maintainable, secure and performant.
Getting your Pull Request Reviewed and Approved
After you defined your Pull Request following the Definition of Done criteria. you can tag any of the member that are in charge of the code review process. This is most likely the Diffgram Core Team or any experienced contributors that have been granted reviewer permissions on the project.
Depending on the areas that your request change, the PR will need to be approved by one or momre project maintainers.
Approval Guidelines
If your change includes change to the frontend backend code it must be approved by the core team or an frontend/backend experienced maintainer. Please review to the Diffgram Core Maintainers list to see which members of the project are the most appropriate to tag.
- If you include changes to existing queries from core parts of the System, at least 2 reviewers must approve the change.
- If you include new e2e tests make sure to include a team member with experience on e2e tests.
- If you PR includes changes to the schema, please add at least 2 reviewers and make sure they have experience the Database schema.
Reviewing a Pull Request
The first thing to make sure that you are correctly reviewing a PR is that you make sure you understand why the existence of this new code is necessary (What problems does it solve, what bug fixes, reasons for refactor, etc)
- Try to be really thorough in your reviews to, you want to reduce as much as possible going through the same code again and again.
- Communicate ideas that you really agree/disagree in the code. Make sure they are clear and easy to understand for other people reading the review.
- Offer alternative implementations if you see there are many routes to go about some specific part of the PR.
- Try to understand the author's perspective, make sure you read any context messages on the commit, comments on the PR or comments in the code.
- If there's something you do not understand, please communicate it. There's a good change other project member might not understand it too.
- Check related issues for more context, and check that there are no depended Pull requests.
- After all your observations, make sure that your summarize if you are overall ok with the code and what specific things you need that might require changes before the approval.
- If there are any required changes make sure to add the
pending_work
tag to the PR.
Responsabilities as the Pull Request Author
The main responsability of the code and implementation of the best possible solution is always of the pull request author. Before requesting a review please make sure that:
- The code you wrote actually solves the problems you are presenting in the PR
- It solves it in the most performant, secure and maintainable way possible.
- It satifies all our Developer Contribution Guide guidelines
- There are no bugs, logical problems, or vulnerabilities with the added code.
Make sure you do a self review of your code before requesting a review of the maintainers to avoid too much back and forth between the maintainers and the PR author.
Updated over 3 years ago