Good Practices for Pull Requests

A good PR guide

  • Pull request for each feature is a minimum when completing a story for the same.
  • Assign a reviewer and setup some time with him , and give adequate time to review .
  • Allocate adequate time for reviewee as well to address the points raised by PR and fix them in the sprint stories.
  • Finally meet once again with reviewer to ‘resolve’ all the feedback given and get approval of the PR.
  • Ensure that the code works and is tested before the merge.
  • Once the PR is approved , validate the build and static code are successful, before merging and delete your branch to maintain repo hygiene.
  Best Practises
  • Keep PR’s small, so that the review is quick and feedback will not take much time.Else most of the time if you have a long living branch with lots of files,its very tedious to go throught various features and requirements to understand what was the expected behaviour.
  • Give a proper description for each PR, with changes implemented nicely listed.
  • Respect the time of both reviewer/reviewee, so that a proper time to fix a time with your reviewer. 
  • As a reviewer, it’s your responsibility not to hold on to the PR for too long , lest you end up blocking deployment and release.
  • Give constructive feedback for each review and not generic statements or mocking .Being rude is not an objective here, even if the code is frustrating to review.
  • The reviewer can ask the reviewee questions to understand more on the functionality or context.
  • Commented code or temp code should be avoided and maintain coding practices including testing and static analysis.
  • Always watch out for sensitive data not being checked into git.Eg config files with password.
  • We should always protect the main release branch from being merged directly without a PR process.

TIP: You now have the ability to review/start a PR within Intellij itself.