MPAS Ocean Review Guide

Reviewing MPAS-Ocean Pull Request

---

When reviewing a pull request to be merged into ocean/develop or ocean/private, please make sure the following are true:

1. You are not merging your own Pull request
2. No hard coded real values occur without the _RKIND specifier. i.e. 5.0 -> 5.0_RKIND.
3. All mpas_pool_get_array and mpas_pool_get_field calls from a pool with multiple time levels have the time level argument. i.e. call mpas_pool_get_array(statePool, 'layerThickness', layerThickness) -> call mpas_pool_get_array(statePool, 'layerThickness', layerThickess, timeLevel).
4. The changes before and after the merge build with all of our supported compilers.
5. None of the commits in the history of the branch touch files outside of src/core_ocean (i.e. git log --graph --oneline --decorate --name-status HEAD only lists files in src/core_ocean).
   1. If a commit does touch files outside, the commit need to be removed. The change cannot be removed in a separate commit.
6. Commits have reasonable commit messages. They should all follow the format below. Specifically, the subject needs to be short, there needs to be a blank line on the second line.
7. The changes follow existing formatting (i.e. tabs vs. spaces and white space at the end of lines)
   • XML and C Files use tabs, please ensure this is always true. Formatting differences make debugging a registry file harder.
   • Fortran files use spaces
8. All of the other cores still build.
9. Features of the ocean are still passing (i.e. bit-reproducibility across decompositions, and bit-restartability)
10. The PR is submitted with the proper format (Following ACME requirements here):
    1. The title of the PR is descriptive of the feature not just the branch name
    2. The description of the PR clearly describes what the PR is intended to do
    3. The pull request is submitted in the correct order (a feature branch should be merged into ocean/develop).

For new features:

1. The PR is accompanied by a design document (Preferably referenced in a comment within the PR, but not in the top description since that goes into the commit message).
2. The design document has gone through the proper review process

Commit message formatting:

All commit messages should follow this format.

<blank line>
body (however long it needs to be)

Merge commits are allowed to have longer subjects if the branch name forces it, but they should still be reasonably short, and the remainder of the message should follow the same format.