Skip to content

Pull Request and Commit workflow notes

Marc Durdin edited this page May 22, 2025 · 31 revisions

Commit and Pull Request messages

Most of this information applies equally to commit messages and pull requests.

  • Messages are broken into title and body. In pull requests, title and body are separate fields, but in a commit message the title is just the first line followed by a blank line and then the body is the remainder of the message (including commit trailers, see below).

Title

fix(windows): re-attach the widget plug which had fallen out

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

Fixes: #1234
  • We use the following commit types:

    • fix: addresses a bug in the codebase. Should almost always have a corresponding bug issue.
    • feat: adds a new feature to the codebase.
    • change: makes a modification that is neither a bug fix nor a feature. You may find docs:, style:, maint:, or refactor: are a better fit.
    • chore: makes a change to code (not infrastructure/CI) which doesn't fit the above categories.
    • maint: makes a change to infrastructure and CI.
    • docs: changes documentation only.
    • style: is a format-only change. No change to code behaviour!
    • refactor: applies for any small or large refactoring of the codebase, renaming variables, restructuring code.
    • test: applies for unit test changes.
    • auto: for automatically-created PRs, such as version increments.
  • The master source for commit types is in /resources/scopes/commit-types.json.

  • If appropriate, include one scope in your title.

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

  • Use the present tense ("add feature" not "added feature")

  • Use the imperative mood ("move cursor to..." not "moves cursor to...", nor "cursor movement", nor "cursor now moves")

  • First letter of title should be lower case ("fix(android): check arguments ..." not "fix(android): Check arguments")

Body

  • Reference issues and pull requests liberally after the first line

  • Explain why you are making the change.

  • Make sure that you detail what is changing in the PR and what the potential impacts are.

See also Pull Request Body below for more.

Commit trailers

  • Commit trailers are used in both commit messages and pull request bodies to add metadata in a consistent way.

  • Commit trailers can be included by adding a blank line at the end of the free-form commit message, and then one or more trailer lines in the format of Trailer-id: content.

  • Commit trailers MUST be at the end of the commit message; in a PR body, they should appear immediately before the "User Testing" section.

Common commit trailers

  • Fixes: #1234 will automatically link and close issue #1234 when a PR merges.

  • Fixes: KEYMAN-MODULE-XYZ will automatically close Sentry issue KEYMAN-MODULE-XYZ when the commit reaches GitHub.

  • Cherry-pick-of: #2468 should be used for a commit/PR that is cherry-picked to another (usually stable) release.

  • Co-authored-by: Name <email@example.com> should be used someone else worked with you on the commit/PR.

  • Relates-to: #1234 can be used to link a related PR or issue, without closing it.

Tips for commits only

  • Try and make your title 100 chars or less, and wrap subsequent lines at 72 chars. 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').

We may choose to push branches to github that we are using for temporary or testing purposes only. For these branches, start the branch name with temp/ so that it is clear that it in not a stale branch that needs attention (suggest that these branches will be deleted after 3 months).

More on 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 or put the PR back in draft to help others.
  • Consider asking your platform advocate to write a unit test for your changes (pair code may work well here)

Pull Request body

  • Include commit trailers in the body of the initial PR comment, immediately before the 'User Testing' section.
  • Explain why you are making the change.
  • Make sure that you detail what is changing in the PR and what the potential impacts are.
  • Make sure you copy out stack and error details from Sentry, because Sentry reports disappear after 90 days!
  • 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).
  • If you make any changes to the user interface, or add new user interface, please include a screenshot (before and after screenshots can be helpful too, if that is appropriate). Similarly, if user workflow changes, please explain that (and make sure you update user documentation in the PR as well).

Pull Request Reviews

  • When a PR is ready for review, you can request a review using GitHub. Make sure you ask for reviews from anyone who you want to review who is not automatically requested via CODEOWNERS.
  • If a PR has been forgotten for a few days, you can share a link to the PR in Slack #general and include the :stampy: emoji. Be witty.
  • Similarly, if a PR is urgent, share a link in Slack #general and include both :stampy: and :fire: emojis.
  • When you start a code review, assign yourself to the PR to notify others that you are taking responsibility for reviewing. This may not be necessary for very quick reviews, but should always be done for longer reviews.
  • If you want to review, and someone else is already doing so, coordinate with them so that we don't end up with duplicate reviews.

Tip

  • You are responsible for watching notifications for pull request reviews.
  • You should make an effort to review other peoples' PRs within 8 "business" hours of the request. In that way we each keep the project moving forward.

Requesting User Tests

See https://github.com/keymanapp/keyman/wiki/User-Testing-Workflows

Flowchart of PR Workflow

flowchart TD
    %% PR Creation
    A(Create PR) --> B{Is it Work-in-progress?};
    B -- Yes --> C(PR in Draft);
    B -- No --> D(Ready to review);
    C --> E(Author makes updates)

    %% CI Test
    D --> F{Did CI test build pass?};
    F -- No --> E

    %% keymanapp-test-bot
    F -- Yes --> G{Are User Tests required?}

    %% Reviewers
    G -- No --> H{Do reviewers request changes?}

    %% User Testing
    G -- Yes --> J{Did User Testing pass?}

    H -- No --> I(Approved)
    H -- Yes --> E
    J -- Yes --> I
    J -- No --> E
    E --> B

Loading

Metadata

Labels

Tip: Most labels should be applied automatically.

  • 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

Tip: Milestone on PRs should be applied automatically.

  • 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.
  • We are experimenting with squash commits on shared-sites repository.

Creating Issues

When creating a new issue, there are two templates to choose from, as well as the option to start with a blank issue. The templates, bug or feature, are helpful in ensuring that all the necessary information for the issue is included.

Issue Title

When creating an issue title, adhere to the following guidelines:

Issue Title Format:

<type>(scope): <concise description of the issue>

The type and scope are defined in the commit guidelines. Focus on the following for the title:

Be Specific and Descriptive: Clearly state the nature of the problem or bug without being vague or ambiguous. Use precise language that accurately reflects the issue.

Avoid Generic Terms: Instead of using generic phrases like "not working", "not functioning," or "not predictable", provide specific details about what is not working or functioning.

Limit Unnecessary Details: For example, do not include the operating system (OS) in the title when the scope already identifies it. The Keyman version number will be included in the detailed description and does not need to be in the title.

Issue Project

We now add issues to the Keyman project automatically (on the main keyman repo at least) and we try to fill in the Complexity and Priority fields. Don't stress too much on the detail here, just fill in your best guesses, and update as estimations become clearer.

Emoji

Emoji are used mainly for visual reminders for humans. On status.keyman.com, the emojis are used to group related PRs in 'project' view.

  • 🏠 is added automatically when a PR targets stable-17.0 by one of our GH actions
  • 🍒 is appropriate for cherry-picks (should normally be a cherry-pick from master --> stable, not the other way around)
  • 🐡 is used by status.keyman.com for KeymanWeb PRs to indicate that the PR increases the artifact size significantly (so we should be verifying it before merge)

Other emoji are mostly used for short-lived projects or long-lived epics, and you can pick any emoji that you like for those.

Clone this wiki locally