-
Notifications
You must be signed in to change notification settings - Fork 148
GSI: How to Make Changes
This document outlines the process required to make changes to the NOAA-EMC/GSI develop. Please refer to GSI Code Management Policy for a higher level discussion.
Once a new task has been identified, go to https://github.com/NOAA-EMC/GSI/issues and open a new issue. Fill in the form fields as appropriate. Assign the issue to yourself or the lead developer for the work outlined in the issue.
An issue is considered inactive if it has not been updated within the past six months. Inactive issues will be closed by the Handling Review Team.
If the forked git branch associated with the issue does not exist, create it in your fork of NOAA-EMC/GSI. Add a comment to the issue with a link to this branch. Include the issue number, XXX, as (#XXX) in commits to the branch. This ensures the issue tracks all changes to the forked branch.
As you work on the issue the authoritative NOAA-EMC/GSI develop will likely change. Please keep your forked branch(es) in sync with the authoritative NOAA-EMC/GSI develop. Include in the issue documentation of significant changes to the code and tests performed. This serves both as a convenient reference for you (like an online notebook) and as a way of giving visibility of what you are doing to other interested parties. Figures and other documents may be attached to the issue and referenced.
Changes committed to the forked branch need to follow GSI Code Standards.
Before a pull request (PR) is opened the changes need to be tested. All changes have to be tested using the standard suite of regression tests. Regression test results must be documented in the issue.
If the results from the new branch are not bit identical with the authoritative NOAA-EMC/GSI develop, reasons for the differences must be documented and the differences confirmed to be acceptable. Confirmation of acceptability may require additional tests and documentation.
Open a new Pull Request to request review and, pending approval, merger of your changes into NOAA-EMC/GSI develop. Please identity the associated issue in the PR. This should be done both in the text of the PR as well as linking the issue via the Development setting on the right hand side of the PR.
At least two reviewers must be assigned to each PR. As a courtesy please ask a potential reviewer if (s)he is available to review your issue before assigning them. If you are unable to assign peer reviewers to your PR, please pass the github usernames of your peer reviewers to the Handling Reviewer for your PR. Please note that those on the Handling Review team can not serve as peer reviewers.
The reviewer’s job is to look at the changes that were made to the branch to ensure the code does what is expected and that the coding standards are adhered to. The reviewer should also ensure that all required tests have been done and results documented. If the reviewer has any questions or requires further tests these comments are made in the review. The developer responds. The reviewer examines updates in response to review comments. This interaction continues until reviewer and develop agree upon the changes. All merge requests require the approval of at least two reviewers. If changes are committed to the branch as a result of the review process, the regression tests should be rerun.
A Handling Reviewer will self assign her/himself to the PR. The Handling Reviewer will ensure the above steps have been followed and documented in the originating issue or PR. The Handling Reviewer may request additional documentation or changes to the PR. A PR may be rejected at the discretion of the Handling Reviewer for reasons including but not limited to size/scope being too large or changes that do not follow current conventions.
Developers are encouraged to submit smaller PRs. This facilitates easier reviews and allows for more agile development. A PR is considered inactive if it has not been updated within the past six weeks. Inactive PRs will be closed by the Handling Review Team.
Once a PR is reviewed it enters the queue of approved pull requests. PRs are processed based on receipt and priority. Receipt simply means that older approved PRs are usually processed before newer approved PRs. Some PRs contain high priority development which must be merged into NOAA-EMC/GSI to support implementations. Such PRs will be acted upon in a manner consistent with their priority.