Pull Requests

DARMA-tasking\LB-analysis-framework developers must submit a pull request (PR) to change code in the DARMA-tasking repositories. The following describes the workflow for code development:

Open an issue

An issue must be open before any work starts on LB-analysis-framework. 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 or 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.

Example commit message:

#123: - implement tags for message delivery
- update readme

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

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
   • Each PR is must be reviewed by a developer on the LB-analysis-framework team. Without an approving review, a PR cannot be merged onto a release/development branch (GitHub is configured to disallow this). A good review must consider a range of qualities in the PR:
     • A clear description of the motivation for "why" the work was performed---referenced from the associated issue.
     • Check that all code style guidelines are followed (for the ones that are not automatically checked)
     • Ensure code works as described and fits in the codebase
     • Ensure that all licenses are obeyed and properly included if new external packages are included
     • Ensure that the concurrent programming model matches with the rest of the codebase
     • Ensure that all interfaces/APIs are documented clearly along with updates to the manual/wiki if relevant for the change
     • Ensure that the code is scalable---has very good isoefficiency. Must not include any unscalable O(P) data structures.
     • Ensure code is readable, extendable, and does not cause bad interactions with other components or use cases
     • Unit/integration tests must be included with the PR that clearly tests the functionality and exercises it sufficiently to be confident that the functionality is being tested.
     • If the PR is addressing a bug report, a test must be included that reproduces the bug (fails or triggers the bug). Without a failing test it isn't clear whether a "fix" is actually correct.
2. All required tests must pass
   • There are now two types of tests: Unit Tests and Acceptance Tests
     • Unit Tests are checking for low level logic - functions, methods, classes
     • Acceptance Tests are checking for correctness of business logic - based on input data test is checking for expected output