Versions Compared

Old Version 2

changes.mady.by.user Bhavin Thaker

Saved on

compared with

New Version 3

changes.mady.by.user Bhavin Thaker

Saved on

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

  • 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) A Committer 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.
      b) A Committer 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 developed by self. A committer separate from the contributor must merge the code to ensure a clean development process.