Skip to content

Pull Requests

Jump to bottom
Jonathan Lifflander edited this page Jul 9, 2020 · 10 revisions

DARMA\vt developers must submit a pull request (PR) to change code in the DAMRA repositories. The following describes the workflow for code development:

Open an issue

An issue must be open before any work starts on DARMA. The issue might be a bug report or a task to implement a feature or enhance a feature. Please include details such as the motivation for a feature request or detailed information on how to reproduce a bug.

Once the issue is created, a unique number will be generated to refer to the issue. For example, an issue might be created with the number #123. In a PR or another issue, one may refer to that issue by typing #123, which will cause GitHub to automatically create a link to that issue.

If the work associated with an issue is planned to go in a particular release, it should be tagged with that release. There are labels that correspond to each release version that may be added to an issue and PR.

Create a branch labeled with the issue

Once an issue is open, developers should create a branch in the repository that corresponds with the aforementioned issue. The branch name follows the format of <IssueNumber>-<short>-<description>. For the previous example, the branch would be named: 123-new-feature.

A branch in development may be pushed incrementally as development commences to the GitHub repository remote and force-pushed (history rewritten) if appropriate for the work.

Commit formatting

Every commit on the branch must follow a specified format for it to be acceptable to merge. The commit must start with an issue number so it can be referenced easily later on during bisection. Followed by the commit number and a colon, a developer can optionally provide a category tag followed by a colon that indicates which component or part of the code that is being worked on. After this, a short message about the commit must be provided.

After the first line of the commit message that contains the issue number, optional tag, and short description, a long description may be provided that provides much more detail about the commit.

Here's a reference that discusses commit message etiquette.

Example commit message:

#123: active: implement tags for message delivery

Implement two types of tags on the active message envelope that
can be used to control message delivery in the active messenger...

Open a PR in draft mode

Once the new branch is in a state where it might be useful to get feedback, create a PR in "draft mode" that corresponds to the issue. Name the issue starting with the issue number, such as: "123 Implement new feature". The labeling system is very important to follow because the automated release tracker link PRs with issues using the description of the PR.

Opening a PR early can be useful to solicit feedback from other developers before the implementation is complete. Also, once a PR is open, the automatic CI testing will be applied to that branch as it gets updated which may be very helpful for development.

Move PR out of draft mode

Once the implementation is complete, take the PR out of draft mode. At this point, the developer should select reviewers for the code on the branch. GitHub will automatically suggest some reviewers depending on who has recently worked on that part of the code.

Merging the PR

To merge the PR, there are several requirements which are enforced by GitHub.

  1. At least one approving review must exist
  2. All required tests must pass
    • The CI testing is extensive and covers many targets. All the targets marked "required" must pass before the PR can be merged
  3. All required checks must pass
    • Three required formatting checks
      1. git --check must pass on all new commits
      2. The trailing whitespace checker must pass indicating that no new trailing space has been added
      3. The commit style will check that all commits messages conform to the required style
    • Codacy analyzes the commits to check for quality problems in the new code
    • Code coverage for the new development will be analyzed and must remain close to the level of coverage before this PR

Once all these checks/tests pass and an approved review exists, anyone is allowed press the "merge" button to finish the PR. GitHub is configured to only allow code that is up-to-date with the target branch (such as "develop") to be merged. If the code is out-of-date, it must be rebased on the current branch (with all tests being re-run) before it can be merged.

Clone this wiki locally