...
- Make sure the PR title and description follows the standard described in Development Reference#Submittingapullrequest(PR)
- EDIT the PR title and description in github as needed or ask the developer to fix it. Work with developer to craft the PR description.
- If the developer has not done so, label the pull request with appropriate component labels on GitHub (i.e. scripts, machinefiles, land, atmosphere, etc...).
- If the branch is fixing a bug, make sure the issue # for the bug is mentioned in the PR description as "Fixes #NN" and the "bug fix" label has also been applied to the PR
- If the developer did not state what testing was run in the comments, ask them. They should at minimum run the e3sm_developer test suite on their branch OR run the 2 test cases used for porting if e3sm_developer is not available AND add new tests for new features if appropriate. You can run the developer test on their branch for them if you wish.
- Make sure the BFB (non-BFB, CC) status of the PR is noted in the description and the same github label is applied.
- If the PR is updating a submodule, make sure all of the developer's changes have been merged to the submodule repo first: e.g. developer's changes in their fork were merged into main submodule repo.
If you are satisfied the branch has been tested, conduct Phase 3 of the code review. See Development Reference#IntegratorCodeReview(Phase3of)Your group may have additional requirements for the code to pass review. You may ask specific people to help review by adding them to the PR with the "Reviewers" pull down menu.- Merge feature branch into "next" branch: (If the target for the branch is maintenance branch, skip to "Maintenance Branch Integration" below)
- Start with your clone of E3SM on a machine you are comfortable developing on.
- Checkout the branch you are going to merge.
- git checkout author/branch-name (note that you don't need to include "origin" in the branch-name. Git will automatically set up a tracking branch.)
- Look at the code changes either on github or using: git log --reverse -p master..
- Assuming tests pass, do "git checkout next"
- Update your version of origin/next with "git fetch origin"
- Update your local next with "git reset --hard origin/next"
- Do the merge with: "git merge --no-ff author/branch-name".
- If there were no conflicts, edit the commit message to follow the Commit and PR message template.
- If there were conflicts, try to resolve them: If you are unsure how to proceed, ask for help!
- List the problem files with 'git status', edit to fix, commit the changes with "git commit -a" which should complete the merge.
- Instead of fixing conflicts during the merge, you could ask the developer to rebase the feature branch to the head of master. Don't merge master in to the branch just to resolve conflicts. See Development Reference#AvoidRoutineMergesFromMaster
- If you can't fix the problems yourself, try working with the developer as outlined in 126846523
- Verify the merge was performed correctly: DAG is as expected, compare the new merge to what was on next before to make sure you are creating the history you think you are, etc... Options include:
- git log --graph --oneline --decorate next (add "--no-color" for a dumb terminal)
- git log --graph --oneline --decorate next origin/next
- git diff origin/next..next
- git log --graph --oneline --decorate next (add "--no-color" for a dumb terminal)
- Run the e3sm_developer test suite on your local merged next before pushing, to verify the merge was done correctly.
- If next is open, push the changes to the main repo with "git push origin next"
- Wait for automated testing harness to run the Integration Test Suite (nightly) on next and confirm that all tests still pass. Results are on http://my.cdash.org/index.php?project=ACME_Climate.
- If the integration tests DO NOT pass, determine if the fails/diffs are due to expected baseline differences OR unexpected differences and/or test failures. Information about the test suites is located on the pages Using the E3SM CDash Dashboard and Interpreting test results.
- IF ALL DIFFS ARE EXPECTED (because answers changed or you added/changed a test)
- proceed to step 5 and merge to master.
- On the same day you merge to master, file a request to bless the tests at https://acme-climate.atlassian.net/servicedesk/customer/portal/2.
- You must merge to master the same day that the next-testing diffs were blessed. This is because we now use the same baselines for next and master. If you don't merge to master on the same day the baselines are updated, master will report a fail for tests run that night because its using the old code with the new baseline
- merge and then request a bless in that order because it removes the possibility of a bless happening without a merge and for namelist blesses it's helpful to have the PR on master.
- For unexpected diffs or test fails.
- If a bug is found quickly, fix the bug, commit the fix to the feature-branch, re-merge the branch to next and repeat step 4
- If you can not fix the problem quickly, revert the merge commit and work on a fix. You can then start again at step 3. NOTE ABOUT REVERTING: Care needs to be taken with any branch that has had a merge reverted on next (or any integration branch). In a subsequent merge of the same branch (e.g., you merge to next, find a problem, revert the merge to next, add new commits to the branch to fix the issue, merge to next again), git will exclude from the new merge any commits it already sees in the history of next (even though they were subsequently reverted), which can lead to a very confusing 'partial' merge of the updated branch. There are two possible solutions to deal with this. 1. Revert the revert on next before re-merging the branch to 'reactivate' the reverted commits. See https://git-scm.com/blog/2010/03/02/undoing-merges.html, "Reverting the revert" for details. 2. rebase the branch so all the hashes change. This prevents git from matching the commits on the branch to the otherwise identical commits that were previously merged. (An interactive rebase where you simply change the commit message by a character in the first commit would also satisfy this criterion.)
- If the branch is fundamentally flawed, revert the merge commit to next, close the PR (on github) and work with the developer to decide if the branch should be deleted.
- IF ALL DIFFS ARE EXPECTED (because answers changed or you added/changed a test)
- If tests DO pass, proceed to step 5.
- Testing can only be skipped for a "hot fix" or for code outside the component models IF the code can be verified by inspection to be correct and have no side effects. Add "Skipping Integration Testing" as a comment in the PR on github.
- If the integration tests DO NOT pass, determine if the fails/diffs are due to expected baseline differences OR unexpected differences and/or test failures. Information about the test suites is located on the pages Using the E3SM CDash Dashboard and Interpreting test results.
- Merge the pull request to master.
- checkout master with "git checkout master"
- Update your version of origin/master with "git fetch origin"
- Update your local master with "git reset --hard origin/master"
- Do the merge with "git merge --no-ff author/branch-name"
- If there were no conflicts, edit the commit message to follow the Commit and PR message template.
- If there were conflicts, try to resolve them: If you are unsure how to proceed, ask for help!
- List the problem files with 'git status', edit to fix, commit the changes with "git commit -a" which should complete the merge.
- If you can't fix the problems yourself, try working with the developer as outlined in 126846523
- Verify the merge was performed correctly (in terms of the DAG): git log --graph --oneline --decorate master (or 'git log --graph --oneline --decorate master --no-color')
- If you are 126846523, push the changes to the main repo with "git push origin master"
You may use the github automerge button (the green button) only for the merge to master. Edit the the title and commit message to match the Commit and PR message template. The default title does not follow ACME conventions.
- If the PR changed answers, either climate changing or roundoff, record details of the PR in the table on Answer-changing commits.
- Delete the feature branch after the merge has been completed. This does not have to be done immediately. You can use the "Delete branch" button on github.
...