-
Notifications
You must be signed in to change notification settings - Fork 64
Development Workflow
This page describes the development process for contributions to the CCPP framework from start to finish.
- Open an Issue
- Develop new feature or bug fix
- Test! Test! Test!
- Submit Pull Request to develop branch
- Code Merged to develop branch
- Merging code to main branch
All development, whether for a new feature, bug fix, re-engineering task, or documentation or data update, MUST begin with opening an issue. An open issue allows for transparency, developer and community input, assignment of the task, and progress tracking.
To open an issue, follow these steps
Coding should take place in a branch in your GitHub fork. If you have not yet created a fork of ccpp-framework, you can create one at https://github.com/NCAR/ccpp-framework/fork by selecting your username from the "Owner" dropdown menu.
A number of tests are required to pass prior to a PR being merged, and it is highly recommended that you run as many of these tests as possible prior to opening your PR.
CI tests (found in ccpp-framework/.github/workflows/) are set to run automatically once a PR is opened. These are mainly unit tests for exercising various parts of the framework system, but there are also "smoke tests" for certain components, such as the ccpp_track_variables.py script. More testing may be rolled into continuous integration in the future.
Tests specific to "prebuild" (ccpp_prebuild.py) are contained in the top-level test_prebuild/ directory. Other tests, including those specific to "capgen" (ccpp_capgen.py), are contained in the top-level test/ directory. To run these tests, navigate to the appropriate directory and run the test scripts; in test_prebuild/ this script is ./run_all_tests.sh, in test/ these scripts are run_fortran_tests.sh and pylint_test.sh. You will need to have all of the framework prerequisites installed (Python 3.8 or higher, a compatible C and Fortran compiler, with MPI enabled) as well as cmake, make, and the python package pytest. When running pytest, you'll also need to set the PYTHONPATH environment variable to include the $ccpp-framework/scripts and $ccpp-framework/scripts/parse_tools directories. You can refer to the .github/workflows/python.yaml configuration file on how to run the pytest tests.
You may find, in the course of your development, that you want to update and run a single test. If the test is a python test, you can run it via pytest. If the test is a fortran test, it will contain a “run_test” script. To preserve the generated files when running the test, run with “./run_test --cleanup NEVER” and the build files will be kept in the associated build directory (for example, in “at_build” for the advection test).
Because the CCPP framework does not generate data on its own except to create software caps, regression testing is done in the context of a host model utilizing the framework. When opening a PR to the develop branch, you should ensure regression testing is passing (or at least, has expected differences with no errors) in one or more host models with your latest code. It is not necessary that all regression tests be run for a given PR (this will be done when contributions are merged from develop to main), but all tests that are run must pass.
The CCPP Single-Column Model (SCM) has a regression test that is run automatically on all PRs that are opened to the SCM repository. To run the tests manually, you can reference the GitHub CI file for running the tests
The UFS Weather Model has a regression test documented here: https://github.com/ufs-community/ufs-weather-model/wiki/Running-regression-test-using-rt.sh
CAM-SIMA regression testing is currently run manually. It is documented here: https://github.com/ESCOMP/CAM-SIMA/wiki/CAM-SIMA-testing in the “Regression Testing (Manual)” section.
To submit a pull request, use GitHub's "New pull request" button and set the proper "compare" (your development) and "base" (the branch on this repository to which to merge your development). All development should be committed to the develop branch outside of special cases (like release branches).
- Enter a short description in the title bar
- In the description text box, enter a longer description with information about the change.
- Using proper syntax, enter the issues which are resolved by this pull request (e.g., “fixes #42”). If there is no issue, create one to describe the problem this PR is solving.
- Add appropriate labels
- If you do not have permission to add labels, list the labels you’d like to include in your PR description
- Developers listed in the CODEOWNERS file will automatically be assigned to review your Pull Request. If your contribution should be reviewed by anyone else, assign those reviewers manually
- Be sure to check in on the PR regularly to respond to comments/questions/reviews!
In order for a PR to be approved for merging to the develop branch, it must receive at least one approval from a developer representing each dynamical core: UFS, CAM-SIMA, and NEPTUNE. Reviewers should review open pull requests as they are able, but if a PR has gone a week or more without receiving the necessary reviews, a code manager may assign reviewers. If you are assigned as a reviewer of a pull request, please review the code as soon as possible. If you feel that you do not have the time or expertise to review the PR, you may remove your name from the list of reviewers unless you are the last reviewer from your organization. One person from each participating organization must review each PR.
After a PR has received the required approvals, and there are no additional requested changes or failing tests, the PR author or a code manager should merge the PR to the develop branch. If the PR author does not have permissions to merge the PR, the code manager should perform the merge as soon as all criteria are satisfied. Otherwise, the PR author (or other designated person if the author is unavailable) should merge the PR once all criteria are satisfied. If an approved and ready PR has not been merged after several days, the code manager may step in and perform the merge. If, for some reason, the PR author does not want the PR merged right away, it should be converted back to a draft; however, if any changes are made, the PR must be re-approved before merger.
Merges should be performed via the "Squash-and-Merge" method. This method keeps the commit history concise–with only one commit per pull request–allowing for an easy-to-navigate code history. It also makes subsequent merges to the main easier (see next section).
The person who merges the PR should manually close any linked issues that are resolved by that PR.
After a PR is merged, the changes have been incorporated into the develop branch. However, most host models will not point to the top of develop directly, but rather to tags or hashes from the main branch. This main branch is updated occasionally (on a weekly-to-monthly basis) when a stable hash is available that can pass required tests (regression tests and any others as specified by each host's code management policies). These changes should be made via a pull request from the develop branch to main. However, when merging a PR for these main-branch updates, the traditional merge strategy should be used, to preserve the history of each pull request as a separate commit in the main branch.
The develop branch represents the "latest and greatest" CCPP Framework developments. While the top of develop should have passed all framework-specific tests, it has not necessarily been tested with all host models. Because it is a heavy lift to run all regression tests for the hosts, especially the UFS Weather Model, the main branch will only occasionally (on a weekly-to-monthly basis) be updated with the latest contributions to develop by a code manager. At this point, all unit tests and regression tests should be passing.
Dynamical cores making use of the CCPP Framework may desire stable tags in the framework repository from time to time; e.g. for simplifying documentation and/or code development. These tags may be created by any developer with write access, so long as they follow the standard naming convention “YYYY-MM-DD” (for tags on the main branch) or “YYYY-MM-DD-dev” (for tags on the develop branch).
Dynamical cores making use of the CCPP Framework may desire release branches from time to time, to coordinate and simplify public code releases or other major code base events. These may be created with the approval of code managers at the regular CCPP Framework developers’ meeting. They should follow the naming convention [dycore]_v[version], where [dycore] is the accepted abbreviation for the relevant dycore (see below) and [version] is the numerical version of the release, in “#”, “#.#”, or “#.#.#” format, as desired. Fewer digits is preferable, since minor releases should ideally stem from the same branch as a major release.