Skip to content

Pull Request and Commit workflow notes

Jump to bottom
Darcy Wong edited this page Apr 17, 2023 · 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.

  • Use the present tense ("Add feature" not "Added feature")

  • Use the imperative mood ("Move cursor to..." not "Moves cursor to...")

  • Reference issues and pull requests liberally after the first line

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

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

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.
Clone this wiki locally