Versions Compared

Key

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

Reviewing other people's code is one of the most valuable contributions you can make to Bahmni (or to any open source code). You do not have to be a senior developer, or a core team member, to make a meaningful code review contribution, and doing a review does not necessarily mean you need to be the one to merge the code. Doing a partial review can still allow the contributor to iterate and improve the PR before a final review and merge happens.


Please see the Submitting Pull Request page for PR guidelines, which will help you to guide the requester

Before you (reviewer) start

  1. Is there a JIRA card created with description of the feature/issue and acceptance criteria articulated in the card? 
  2. Is the PR marked with the right JIRA card number? When you merge, you must ensure that the JIRA card number is reflected in the commit msg.
  3. Are you the right person to review this PR? If not, try to direct to the right person. If you have partial context, try to find help from one who knows better.
  4. Are any checks (travis-ci, codeclimate) failing? If so, immediately request that the contributor look into this.
  5. Has the submitter signed the CLA? (See the status of the cla-assistant check.) If not, kindly ask them to do so.
  6. Ask questions, instead of making statements. Avoid “why” - which often seem accusatory, rather try to understand the context and reasoning … “what was the reason for ….”
  7. For bigger PRs, try to “meet” with the person. Often pairing on the PR review for big PRs is the best way. Bigger PRs are also best to merge onto a branch, run/check locally and then commit.
    1. You can follow github ‘Command line instructions’ suggestions to merge a PR locally before pushing onto master. Often for large PRs and in cases you can’t assert the correctness of the code without running it locally, this is a preferred approach. Just go the PR link in github; towards the end click “command line instructions” next to the “merge pull request” button and follow the suggestions.  
  8. Lots of handy tools exists for code analysis - e.g. a Static Code analyzer with IntelliJ or Sublime

...