...

• When working on a large project, try to break down contributions into small logical blocks that can go into separate PRs and therefore can be merged incrementally. Avoid creating a single large PR with a huge number of changes.

• Include an appropriate title and description for your PR. Include links to github issues that are pertinent to this PR. Use the description to inform the reviewer of any details that may be hard to infer just from looking at the code diff.

• When adding new APIs or modifying existing APIs:

• Make sure to add/update appropriate API documentation.

• If adding a new API, include in the API documentation sample code that shows how to use the API.

• If modifying an existing API, make sure that API documentation and code samples are still accurate after your changes.

• Changes that add new functionality or extend existing functionality should always be accompanied by unit tests that exercise that functionality.

• When updating code that is performance sensitive, include in the PR description details on how you tested your changes from a performance standpoint.

• Please @mention those individuals who you think might be interested in reviewing your PR or otherwise can provide meaningful feedback.

• If you are actively working on a PR and therefore not immediately looking for feedback, include a [WIP] prefix in the title.

• If the PR build fails, it is your responsibility to understand why it failed and fix the underlying issue. As a best practice, always ensure you can build your changes locally and that there are no lint errors before opening/updating a PR.

• When you receive feedback for your PR, try and address the reviewer’s comments/concerns in a timely fashion. If you don’t have time to immediately address them, adding a short note to the PR about when you plan to update the PR will be helpful. Feel free to solicit help if you want others to pitch in.

Guidelines for Reviewers/Committers

• As a reviewer you have the responsibility to ensure contributions meet the quality bar. This means you should try and ensure:

• Contributions are inline with the overall code architecture.

• Contributions are not lowering the overall code maintainability i.e. documentation and unit tests are not missing.

• If you don’t understand the logic/flow now, chances are it won’t automatically get better in the future. So feel free to push back if code/logic is hard to follow. Ask to include comments clarifying implementation details whenever necessary.

• When providing feedback or when seeking clarification, be clear. Don’t assume the PR owner knows exactly what is in your mind.

• If you decide to close a PR without merging, get an ack from the PR owner that that is indeed the right next step.

• Proposed NOT agreed upon yet: 

  • A Committer should NOT delete a comment from any contributor. But in the rare case, a comment has to deleted, it must be approved by majority of the committers.

  • A Committer must NOT self-merge any code. If it has been reviewed by another committer, a comment must be added saying so OR it is better to have the reviewer merge the code to ensure a clean process.