PR Guide

PR Guide

Created
Dec 8, 2021 6:10 AM
Department
Engineering
Category
Technology
Git
Tags
GitHubPR
Date
URL

Standard PR and development Flow

  1. Create the feature branch (Let’s say COM-123). Make sure to branch out from main 
  2. Commit your changes
  3. Create pull request into main. The Marvin bot will automatically create the PR for staging
  4. Request for review on the main PR
  5. Wait for approval on the main PR
  6. Merge the staging PR
  7. Test changes on staging
  8. Merge the main PR
  9. Test changes on

Things to keep in mind

  • When merging with staging, if there are merge conflicts never pull from staging to your branch. Create a separate feature branch to push the changes to staging
  • staging should never be merged into any other branch
  • staging and main should be in sync before you can start using this flow
  • Do not use the squash option on the GitHub UI. Instead, squash the changes on localhost and then push the squashed commits. This makes sure that the commit hashes remain the same for both staging and main

Branch Conventions & Commits

  • Use conventional commits.
  • The branch name should be UN-jira-ticket-id, for example UN-325
  • If it is a parent ticket and sub tickets have the same/conflicting/dependent files, then create a branch with the parent ticket but in your commits: Each commit will represent a feature, and your commit message should have scopes : (UN-323) - feat: your commit message

PRs

  • Make smaller PRs w.r.t features.
  • Always use the Create a merge commit option
  • image
  • Conventional commits are being followed for the commit message title, for the body one can →
    • Write the problem which was faced / what you were trying to fix. [ If this is a new feature, this is not needed. ]
    • Break down a list of changes / highlight the changes which you have made.
    • Add a video recording to go along if there are major UI level changes.
    • You can always add screenshots to share better context.
  • Some examples of really good PRs raised by engineers :
    • @Gurkirpal Singh
    • image
    • @Jishnu De Sarkar
    • image

PR Reviews

  • Usually, PRs are raised and requested for code reviews. Here, this could be used for context updates/sharing.
    • For example, let’s say you are raising a PR and you are the reviewer, here you would still request a review from all the developers in the project ( If needed, even from someone who is in the same squad, who is to later contribute here. )
    • You can decide on the threshold time for this to be open and for everyone to have context as to what are the changes. Usually, after 1 day, you can merge this and move forward.
  • For big singular features which need to be interdependent even on small features, one can raise a draft PR as well.
    • The commits will have to represent broken-down features, which reviewers can review and keep a check on.
      • Example : - Assume you are working on branch UN-100, your commits would look like this:
        • (UN-101) : Feat - Your message
        • (UN-102) : Fix - Your message
  • If the review goes beyond the time spent, you can let the reviewer know about this, after this we can decide if the feature can be moved forward with / without the review.
  • In general, whoever is requested a review would be free to →
    • Communicate which ideas you feel strongly about and those you don't.
    • Identify ways to simplify the code while still solving the problem.
      • Example :
      • image
    • If discussions turn too complex or long, please schedule a meeting to talk about this. Inform your PMs and get it on the capacity planning.
    • Offer alternative implementations if you think something better can be done.
      • Example :
      • image
    • Seek to understand the author's perspective.
      • Example :
      • image
    • Remember that you are here to provide feedback, not to be a gatekeeper.

Permissions to be set up

This is to be set by the admins of the repository which involves branch protection rules for the main branch.

image