Skip to content

Pull Request Process

Jump to bottom
Slin Lee edited this page Jun 8, 2020 · 5 revisions

Basics

We don't do anything too strange here. I'm writing it down in one place.

Creating branches and PR

When working on an issue, create a new branch from dev. We use dev like most repos use master

Most of the team uses branches named after the issue they are working on: dev-123-fix-titles

Feel free to open a pull request even if you're not ready for a review yet. Use the 'draft' PR mode or add the labels 'WIP'.

Link the relevant Github issues from the PR. A simple way is to use the 'closes' keyword in the PR description: Closes #123

Code reviews

Tag relevant teammates for reviews. If you're not sure, tag @slinlee, and I'll review or find the right person. It doesn't hurt to have more people look at code.

If you're tagged to review code changes but aren't familiar with the area, it's a good chance to learn about another part of the product. No pressure though if you're uncomfortable.

For comments on code reviews, let the person who created the comment resolve it in Github. It makes it easier to make sure the comment was addressed.

Merging code

New Update - The PR author can merge their code to dev once these requirements have been met.

Requirements:

  1. Needs at least one approval
  2. New tests for the feature should be added. Ping @EvgeniyEA if you need help.
  3. Tests need to be green on Jenkins
  4. If you are making Liquibase/JOOQ changes, make sure you merge in the latest changes from dev and that your DB change ID is the latest, then regenerate any files. Send out a note in Slack on #pathmind-dev when you're merging one of these changes to avoid collisions. Thanks.

PR authors can merge the code and are responsible for debugging and fixing if dev breaks as a result.

I prefer to Squash and Merge PR into dev. It keeps the history linear and is a decent balance between keeping the history readable/clean and ease of development. Rebasing is acceptable it's just one commit or you cleaned up the commits already.

Merging from dev to master will automatically release the changes to production (app.pathmind.com). Slin will be the one who does this. Before this step, I'll do one last pass of the DB migration change IDs.

Of course, we will adjust this as needed. It's always a balance of stability and removing the bottlenecks for developers. I'd love to hear suggestions for improvements. Thanks. -Slin

Clone this wiki locally