Pull Request and Commit workflow notes

Commit messages

• We use Conventional Commits for commit messages, for example: fix(windows): Re-attaches the widget plug which had fallen out

• We use the following commit types:

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

• If appropriate, include one scope in your commit message. These correspond roughly to folder names.

  • android
  • ci -- Continuous Integration; use a platform scope in preference to this scope if possible
  • common
  • developer -- Keyman Developer
  • ios
  • linux
  • mac
  • web
  • windows

• 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.

• 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.

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.

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 a checklist that can be ticked off by the tester (Markdown: - [ ] foo).
• Tag @MakaraSok (lead tester) in the same comment so that he is notified for testing.

Testers: Recording test results

• Add a PR comment detailing the testing undertaken and results.
• Tick off passing tests in the original user testing comment.

Failing tests

• If a test does not pass, add a Request Changes code review (in Files changed area) with details on what failed.
• You might want to use **FAIL:** in your comment to make it more visible that tests didn't pass.
• 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, enhancement, and modification.
• If your PR is a cherry pick of another PR to another branch (e.g. from beta to master), include cherry-pick.
• For PRs that target a stable release, include stable.
• We prefer to leave a PR in draft if it is not ready, but you can mark a PR with work-in-progress if it is no longer in draft but is not ready (e.g. significant work has been identified on it)

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.