MALI Code Integrator's Guide

This page documents the workflow for a code integrator to merge pull requests.

Code review guidelines

• When a PR is ready for review, a reviewer or multiple reviewers should be assigned, and the 'in progress' label should be removed if it had been applied.
• The author should run the COMPASS integration suite on their branch before requesting a review or request help in running COMPASS.
• Ideally each PR is reviewed by at least one person other than the author.
• Often the reviewer will also be the integrator, but that is not necessary. If needed, the author can be the integrator on a small team like this. If so, there should still be an independent reviewer.
• The integrator will also run the COMPASS integration suite on the merge of the branch to develop, which can potentially catch conflict issues that did not come up from testing the head of the branch.
• Code review should check for (this list needs to be elaborated upon):
  • feature functionality
  • that existing functionality does not break
  • readability
  • code following existing MALI conventions (which we need to summarize somewhere)
  • ideally a new COMPASS test is introduced for new features (not currently enforced but should be)

Code integration process

1. Establish a COMPASS baseline using MALI-Dev/E3SM/develop if one is not already generated. See https://github.com/MALI-Dev/E3SM/wiki/MALI-Development-Overview for details. Note it is possible there are test failures on the baseline, though ideally this should not be the case. Use your judgment if those need to be resolved before code integration can occur.
2. In the repo where you will do the merging and testing, check out a local develop branch and reset it to the current version on MALI-Dev (here assuming [email protected]:MALI-Dev/E3SM.git remote is named MALI-Dev:

• If you don't have a develop branch already: git checkout -b develop. If you already have a develop branch: git checkout develop.
• git fetch MALI-Dev
• git reset --hard MALI-Dev/develop

3. Merge the branch from the PR into your freshly updated develop branch: git merge --no-ff remote/branchname The subject of the merge commit should follow this format: Merge branch 'AUTHOR/mali/BRANCHNAME' into develop, e.g. Merge branch 'trhille/mali/restore_thickness_after_advection' into develop. This follows the convention that E3SM uses and will avoid trouble with later merges to the main E3SM repo. Copy the text from the PR description into the body of the commit message. It's ok to also leave the the auto generated commit summary at the bottom of the commit message template.
4. Check git logg (alias for git log --graph --oneline --decorate --author-date-order) that the merge commit brings the PR branch into develop correctly.
5. Run the COMPASS integration suite on your local updated version of develop, running against the previous version of develop as a baseline. See https://github.com/MALI-Dev/E3SM/wiki/MALI-Development-Overview for details. Be sure both the baseline and the test were done with an executable compiled with DEBUG=true.
6. If all tests pass, the PR is ready to complete. If any tests fail, it may either be expected or unexpected failures. If the failures are expected, review them carefully and then if they make sense, you can deem them "blessed". In that case future COMPASS tests will need to use this new test output as the baseline. If failures are unexpected, work with the PR author to resolve the problem. If any changes are needed to the branch, then the test merge should be discarded and the whole code integration process will need to be started over after the PR is updated.
7. To complete the merge, push your updated develop branch to MALI-Dev: git push MALI-Dev develop. Check the github webpage for the PR. It should show a purple "Merged" badge at the top. Congratulations - the PR integration is complete!

Notes

• It is possible to do this entire workflow on a detached head instead of actually creating a local develop branch. If you know what you are doing and feel more comfortable with that workflow, that's fine.
• The GitHub website user interface includes the ability to auto-merge pull requests via a green button. We have chosen to never use this for the MALI-Dev/E3SM repository because it does not allow enough control of the result. By merging on the command line you have more control over the merge commit message and you can review the merge history locally for errors before pushing it and making it "final". The practice of not using the green merge button follows that of E3SM-Project and MPAS-Dev.

Merging upstream changes from E3SM-Project/E3SM into MALI development branch

We periodically want to merge changes that have happened in the primary E3SM repo (master branch in E3SM-Project/E3SM) into MALI's development branch (develop) in this repo. Examples may be changes to MPAS Framework, changes to E3SM build environment, or changes in compsets or other components that MALI interacts with in the coupled model. Ideally, we will only do this once every few months to avoid extra work, but in can happen at any time it is needed.

The process is:

1. Ensure you have remotes to your repository for both E3SM-Project/E3SM and MALI-Dev/E3SM, e.g.:

$ git remote -v
E3SM-Project	[email protected]:E3SM-Project/E3SM.git (fetch)
E3SM-Project	[email protected]:E3SM-Project/E3SM.git (push)
MALI-Dev	[email protected]:MALI-Dev/E3SM.git (fetch)
MALI-Dev	[email protected]:MALI-Dev/E3SM.git (push)

2. Fetch your remotes: git fetch --all -p
3. Check out a local version of our development branch.
   If local develop doesn't already exist: git checkout -b develop MALI-Dev/develop If you already have a local develop branch, either delete it first or instead check it out and reset it to the remote version: git checkout develop; git reset --hard MALI-Dev/develop
4. Merge E3SM-Project/master into your local develop branch: git merge --no-ff E3SM-Project/master. Follow the commit subject line example below, and add a brief justification for why you are doing the merge:

    Merge branch 'E3SM-Project/master' into develop

    Get updated machine files for E3SM available in MALI development branch.

Note: The --no-ff flag ensures that the merge is committed, even when it could be resolved as a fast-forward. Instead of using --no-ff, you can add this to your .gitconfig file:

[merge]
         ff = false

5. Confirm the merge looks correct. You can use the command git logg --merges and confirm that the first parent (previous commit on the left side) is MALI-Dev/develop and the second-parent (previous commit on the right side) is E3SM-Project/master. For example:

*   d6309858d9 (HEAD -> develop) Merge branch 'E3SM-Project/master' into develop
|\
| * 417b03f684 (E3SM-Project/master) Merge branch 'sbrus89/ocn/improve-harmonic-analysis-options' (PR #4586)
| * ... bunch more commits here.
* | 0f8ff9ef09 (MALI-Dev/develop, MALI-Dev/HEAD) Merge branch 'matthewhoffman/mali/damage_advection' into develop

6. Test the merge following standard COMPASS testing procedure.
7. Push the updated version of develop to MALI-Dev, i.e., git push MALI-Dev develop