Backend Code Review¶
These are general guidelines to follow when reviewing backend code. Please feel free to add to them.
Additional Reviewers¶
If you’d like a second opinion before merging the code, please tag another reviewer and ask them to also review the PR.
It’s especially recommended to tag authors of specific features if the feature they built is being touched, or to tag (other) maintainers in case there are architectural changes to existing code or to support new features.
Documentation¶
- If there are any lines that are not immediately understandable to you, ask the author to add a comment explaining the line.
- If you’re not sure why the author chose a particular approach, or if there’s some context on the pull request description that would be useful in the future, ask the author to add it as a comment to the code.
Style¶
Linting should handle many style issues, but here’s some to check manually:
- Imports should be ordered in alphabetical order, with standard library imports first, third-party imports second, and local imports third.
- All class attributes should be defined before class methods.
- Internal functions (not meant to be used outside the class or file) should be prefixed with an underscore (
_
). - Variable names should not be reused within the same function.
- Identifier terminology (for the service layer):
- All the database object identifiers should be named with database object followed by
_oid
e.g.database_oid
,schema_oid
andtable_oid
. - Columns should be identified by
attnum
. - The django model identifiers should be named with model name followed by
_id
, e.g.database_id
,user_id
,server_id
.
- All the database object identifiers should be named with database object followed by
Testing¶
- There should be tests for new functionality.
- There should be tests for both failure and success cases (as applicable)
- New functionality that applies to both the API and the database code should be tested in both places.
User Experience¶
- If there are any user-facing changes to the product that are not directly defined by the GitHub issue, please run them by Kriti.