Skip to content

Pull Request and Commit workflow notes

Jump to bottom
Marc Durdin edited this page Jul 8, 2021 · 28 revisions

Commit messages

fix(windows): Re-attaches the widget plug which had fallen out

Fixes #1234. The widget plug was wonky so I attached paper clips
so it would stay in more firmly.

Commit

  • We use the following commit types:

    • fix: addresses a bug in the codebase. Should almost always have a corresponding issue. GitHub label fix should be applied.
    • feat: adds a new feature to the codebase. GitHub label feat should be applied.
    • change: makes a modification that is neither a bug fix nor a feature. Corresponds to GitHub label chore. You may find docs:, style:, or refactor: are a better fit.
    • chore: makes a change to infrastructure which doesn't fit the above categories. (GH label chore).
    • docs: changes documentation only. Use GitHub label chore.
    • style: is a format-only change. No change to code behaviour! Use GitHub label chore.
    • refactor: applies for any small or large refactoring of the codebase, renaming variables, restructuring code. Use GitHub label chore.
    • test: applies for unit test changes. Use GitHub label test.
    • auto: for automatically-created PRs, such as version increments. Use GitHub label auto

  • The master source for commit types is in /resources/scopes/commit-types.json.

  • If appropriate, include one scope in your commit message.

  • If your commit crosses multiple scopes, consider breaking it into multiple commits. This helps reviewers!

  • Include text "Fixes #1234" in the commit message to automatically link and close issue #1234 when the PR merges.

  • Try and make your first line 50 chars or less, and wrap subsequent lines at 72 chars. The "Fixes #1234" text does not have to be on the first line. This may be difficult to achieve so don't stress too much!

  • Tip: If you use VS Code as your default editor for Git for Windows (an option at install time), then it will turns your text red when it overflows and shows a 72 character margin in the commit message editor.

  • How to setup up a git commit-msg hook to enforce conventional commits

Branch Names

We use the same types and scopes as listed above for branch names. The basic format is:

<type>/<scope>/[cherry-pick/][<issuenum>-][name-kebab-case]

For cherry-picked branches we should add the cherry-pick/ component. If the branch addresses a specific issue, try and include the issue number. Keep names lower case and use hyphens to separate words ('kebab case').

Pull Requests

Workflow - draft and work-in-progress pull requests

  • Push early to a draft PR: it's a backup, it helps with tracking progress, and keeps us thinking about getting our PRs to completion.
  • While in draft, feel free to rebase and force push in order to better organise commits.
  • Once a PR exits draft, do not use force pushes. Force pushes make it hard for reviewers to track changes in response to reviews.
  • If a (non-draft) PR needs significant work, then apply the work-in-progress label to help others.

Pull Request body

  • Include "Fixes #1234" in the body of the initial PR comment to make it more visible to reviewers.
  • Make sure that you detail what is changing in the PR and what the potential impacts are.
  • If you start from a commit message, remember that the pull request body should not need to be manually wrapped (i.e. you may want to unwrap it).

Pull Request Reviews

  • When a PR is ready for review, you can request a review using GitHub, but also share a link to the PR in Slack #general and include the :stampy: emoji. Be witty.

Requesting User Tests

  • In a PR comment, include a ## User Testing section with details on what needs to be tested by our test users.
  • If no user testing is necessary, note the reason, e.g. Not required (dev only).
  • If possible, use an itemized list with a name for each test so we can correlate with test results (Markdown: - <test name>: steps...).
  • Tag @keymanapp/testers (Keyman test team) in the same comment so that they are notified for testing.

Testers: Recording test results

  • Add a PR comment detailing the testing undertaken and results.
  • For each test, add - <testname>: **PASS** if the test passes and add - <testname>: **FAIL** (details)

Failing tests

  • If a test does not pass, add a Request Changes code review (in Files changed area) with details on what failed.
  • Refer to the name(s) of the tests that fail in the code review.
  • After the code is fixed, and the test passes, you will need to add an Approve code review so that the PR can be merged.

Metadata

Labels

  • Make sure your pull request labels correspond to the commits included. We have scoping labels for each platform, a few sub-scopes, and type labels bug, feat, and chore.
  • If your PR is a cherry pick of another PR to another branch (e.g. from master to stable-12.0), include cherry-pick.
  • For PRs that target a stable release, include stable.

Milestone

  • You should almost always tag with the current sprint. In rare cases, you might tag a PR with future. Don't tag a PR with subsequent sprint tags -- they will be rolled over to the next sprint automatically anyway.

Merging PRs

  • Currently, we use merge commits.
Clone this wiki locally