Skip to content

Pull Request and Commit workflow notes

Jump to bottom
Eberhard Beilharz edited this page Jun 27, 2024 · 27 revisions

Commit messages

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

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!

  • 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")

  • Reference issues and pull requests liberally after the first line

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

    • Include commit trailer Fixes: #1234 to automatically link and close issue #1234 when the PR merges.

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

    • Include commit trailer Cherry-pick-of: #2468 for a commit that is part of a PR cherry-picked to another release.

    • Include commit trailer Co-authored-by: Name <[email protected]> if someone else worked with you on the commit.

  • Try and make your first line 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).

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.

Pull Request body

  • Include "Fixes: #1234" in the body of the initial PR comment to make it more visible to reviewers.
  • 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).

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

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

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

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.

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