Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

This document provides details on ACME E3SM development conventions and practices.

...

Table of Contents

Introduction

All ACME E3SM developers should consider themselves stewards of the repository history. Our goal with defining a workflow is to improve the utility of the repository and create a useful history that can provide a tangible benefit to other developers.

...

This document expands on portions of the Development Getting Started Guide providing more detail on ACME E3SM development workflow.  You should read the Quick Guide first, and use this document as needed.

...

important -- Items colored in red mean they are important, and should not be ignored.

one time -- Items colored in green are commands to be issued once per machine.

repo once -- Items colored in orange are commands to be issued once per local repository.

...

Creating a new branch: new feature

New projects development should be carried out on a branch. New projects developments include the addition of a new feature (github-username/component/feature) or fixing a bug (github-username/component/bug-fix).

When creating a branch, please name it Your branch will typically start from master.  It may start from another developers branch or a maintenance branch.   Never start a branch from "next".

When creating a branch, please name it based on guidelines contained in Branch, Tag, and Version name conventions. You should also create a set of baselines for tests you plan to use to verify your code changes, and store these baselines in a location specific to your new branch. This procedure is described in Testing.


NOTE: 

 As seen above, ACME E3SM utilizes a naming convention for branches. Branches have compound names separated by a “/” to describe what large scale part of ACME E3SM the branch changes will modify. The / does not denote a directory, it just is used in naming the branch. This lets other developers know what these branches are developing and what parts of the model they should be modifying. This practice will be referred to as “namespacing a branch”

...

When creating a branch, multiple levels of namespacing are allowed. In ACMEE3SM, a branch should should  always be namespaced first by github user in charge of the branch. Next comes the main component the feature is being developed for.  Finally, a description of the new development. For example, a branch that is implementing a new parameterization within cam EAM would be named:

joeuser/cameam/new-parameterization

And a branch that is modifying the HOMME dynamics would be:

...

  • The bug existed in code imported from CESM such as in the initial version added to the repo.
  • The bug was introduced prior to v0.3 (the version where the testing is working)

Changing Branches


...

Creating a new

...

 branch: maintenance

The E3SM repository will have one or more maintenance branches usually to keep adding bug fixes and machine updates to old versions.   See Existing and Planned Tags and Branches for the list of branches and their purpose.

If one of the maintenance branches needs features added to it, make a branch from the head of the maintenance branch.  You do this by first checking out the branch by its name.

git checkout maint-1.0

This will put your working directory at the head of the maintenance branch.  You can then start a new branch as you do for master.   When you make a pull request, specify the maintenance branch as the "base".

If you need to fix a bug on a maintenance branch and the bug was introduced by a commit on the branch, you can follow the same directions as for a bug-fix branch.

IMPORTANT:  We do not have "next" branches for maintenance branches to test multiple features against each other.  To compensate, all development on a maintenance branch should be done sequentially.  That is, one developer makes their branch, completes the feature, tests it and then has it integrated to the branch before the next development starts.   This should not be a problem because maintenance branches are not supposed to have heavy development.   You should always test feature branches but this is more important for maintenance since there is no "next" testing and you don't want to break the maintenance branch by integrating a broken feature.

Changing Branches

Creating a new branch will not change the current branch that is being worked on in the current working directory.

Because the git branch command does not modify the current branch in the current working directory, new commits will not be made to the new branch. In order to change the branch that is being worked on, the git checkout command is used. For example:

...

Utilizing the repository history

 

Seeing as all of the ACME E3SM developers are stewards of the repository history, it would be useful for developers to understand how to make use of the history they are maintaining. 

...

git commit will open up an editor where a commit message can be written.   Please refer to the Commit message template for more instructions on the ACME E3SM commit message guidelines.

...

  1. Commit all your changes and push the branch to the shared repository, 
  2. go to https://github.com/ACMEE3SM-ClimateProject/ACMEE3SM/branches and find your branch.  Click on  "New pull request".
  3. You should verify base and compare of your pull request are correct.  The "base" is master and "compare" is your branch.
  4. Enter a PR Title:  Make sure the title is 70 characters or less and explains the PR in a "verb noun" format like "add cool new feature". Do not just use the branch name or what is filled in by default.  (If your branch has a single commit, the title of that commit will be the default, but if your branch has multiple commits, the branch name will be the default.)  You should edit the title to make sure it is descriptive enough. DO NOT include github issue #'s or JIRA tasks in the title. Hyperlinking does not work from the title.
  5. Write a PR Description.  
    1. In the "Write" field, provide a descriptive message of what all the commits in the PR will do together.  Do not start it with a phrase like "This PR".  Use the "verb noun" format again.   Repeat the title if necessary.  
    2. If a bug fix:  after the description, close any Github issue numbers for the bugs this commit fixes using keywords like "Fixes #123".  See https://help.github.com/articles/closing-issues-using-keywords/
    3. After the description and any issue numbers,  add a keyword indicating how the PR might change answers.  
      1. [BFB] or [non-BFB] or [CC]:  Add ONE of these keys to indicate if this commit will affect testing results to roundoff [non-BFB] or climate changing [CC]. Use [BFB] if-and-only-f commit is bit-for-bit and you know all the tests will pass without regenerating baselines.
      2. [FCC]:   Add [FCC] if the commit will change climate if a flag is activated
      3. [NML]:  Add [NML] if the commit introduces changes to the namelist.
    4. The reviewer will review the description as part of the pull request. The description will be used in the commit message used when the PR is merged to next and master.     The integrator may work with you to edit the PR description.  Do not include Confluence URL's in the PR description.
  6. Give PR label(s) ('Label' pull down menu).  Add a label for the component this PR involves.  Add the "bug fix" label if this is a bug fix.  Add the label(s) for BFB, non-BFB, CC, FCC, NML as appropriate.
  7. Assign a single Integrator to manage the PR ( use the 'Assignee' pull down menu)
  8. If you want additional reviewers, add them using the Reviewer pull-down menu.  Anyone can be asked to review.
  9. When complete, click on "Create pull request".  This will start the code review and the process of moving this feature to master.  
  10. After pull request is created, add  the following in the Comments section:
    1. In the first comment, provide a link to the Design Document governing this PR.   See /wiki/spaces/CNCL/pages/25231511.
    2. In subsequent comments.  
      1. Provide information to aid the Integrator in running, testing, and validating the feature (but that is too specific to be included in the general PR description). 
      2. Say how the feature was tested.  Example:  "acmee3sm_developer on Titan passed".  If a test is expected to fail and required redoing baselines, state that and list the tests that fail.

Your PR is not finished until it has been merged to master by the Integrator.

This document can be used to help with pull request related issues.

...

  1. Check the github entry for the PR and make sure it has a good title and description, correct labels and a comment with a link to the Design Document.   A PR can not be merged to next or master unless it has a Design Document with Phase 1 and 2 completed. See /wiki/spaces/CNCL/pages/25231511.
  2. Look at the code changes either on github or using:  git log --reverse -p master.. on your checked out copy of the branch.
    1. Does new code hold up to visual inspection for code quality?    Look over code changes for glaring mistakes or code style issues (e.g. useful comments, reasonable subroutine lengths, new code in an existing file follows conventions of that file).
    2. Check to see if the description of the code changes in the PR match the actual changes.  Make sure nothing unrelated to the PR was committed accidentally.
    3. Although they can't be changed, see if commit messages on the branch follow the Commit message template and let the developer know if they can not.  Consider asking the developer to squash commits to clean up the history.
    4. Have tests been added or suggested that exercise this feature?
    5. Does code run on all platforms after integration into next?

If there are any problems, work with the developer to correct them.  All correspondence should be done as comments to the PR in github.

An ACME E3SM group may define additional review procedures for code changes affecting their component.

...

will rebase your branch using the tip of the ACME E3SM master branch as its base. Finally, modifying code might require introducing new commits, or editing previous commits.

...

In addition to testing the feature, it is the reviewers responsibility to verify the pull request does not introduce issues related to the previous functionality of master.  To ensure that the model still passes basic tests in multiple configurations, developers will be asked to run the acmee3sm_developer test suite.  This can either be performed on the branch itself, or in an integration branch. The reviewer will inform the developer where testing should take place.

 Basic instructions for running the acmee3sm_developer test suite can be found at Testing.  Some additional information on testing procedures can be found at /wiki/spaces/SE/pages/7998600 and /wiki/spaces/SE/pages/3244819.  Results from nightly test suites run on the "next" and "master" branches can be viewed on the ACME CDash website.
 

...

If they do happen to run into this situation, you may be requested to help with the process. The easiest way to help is to create a new branch at the HEAD of the branch your pull request was submitted from. This branch should have the same name as the other branch with -resolved appended to the end. After the branch is created, you can merge ACMEE3SM-Climate/ACMEE3SM/master into it, and push the resolved version onto the shared repository. This gives the reviewer a version of the code that is merged and resolved, but allows the reviewer to ensure the history maintains the standards.

...

  • Never merge from master to your feature branch. (In normal development. See NeverMergeFromMaster below for more info.)
  • Never merge "next" to "master"
  • Never use "cherry-pick" on "next"
  • Never do simulations using "next"
  • Never commit directly to "master" or "next".  Only use merge commits of branches.
  • Never rebase commits on your branch after it is merged to next.
  • Never start new development from "next".

Changing the url for your remote

...

Before communicating with remotes, you might want to add or remove remotes. In order to add and remove remotes the git remote command can be used. The two uses are as follows:

git remote add remote-name protocol:address/to/repo # Creates a remote


git remote remove remote-name # Removes a remote

  

In order to communicate with remotes, there are three actions. pushpull, and fetch.

...

can be used to write a local branch to a remote branch on the remote pointed to by the name provided. An explicit example would be: 

git push ACMEE3SM-ClimateProject/ACME E3SM feature:github-username/component/feature

...

You never need to "merge from master" to make the eventual merge of your feature branch "easier".   This is because, unlike svn, git is smart enough to ignore changes that have occurred in files you aren't working on when you finally merge your branch to master.

This policy is to make it easier on developers - developers dont have to concern themselves with what a hundred other things going on in ACME E3SM and can focus solely on their feature.  The integrator will evaluate how this feature works with the latest version of ACME E3SM when they merge the feature to 'next', before it is merged to 'master'.   We also don't want unnecessary merges from upstream since they just clutter and confuse the repository history and provide more opportunity to break things in the branch. However, there are cases where merging from master (or better, an upstream release) can make sense - such as a long lived development branch.  The rule of thumb for any merge is that if you can clearly describe what you are merging and why that merge is necessary for your branch to be completed (so that it can be merged upstream), then it's fine to merge.  But merges without such justification are generally undesirable.  (See discussion here )

If you want to see how your feature works with the latest version of master, do a test merge.  Update your local version of master,  make a new branch called something like "testmaster" and check it out.   Merge your feature branch to this new branch (which is just a copy of master) and run your tests.   When you're done, you can delete the "testmaster" branch.


Details:

One of the main differences between git and svn is how history and branches are stored. In svn branches are virtual directories (i.e. svn copy ^/trunk branch_name) and the history is stored as a stack of revisions (providing the monotonically increasing revision number). The problem with this is that a branch isn't actually a branch. It doesn't contain the information about where it was created from or what has been "merged" into it. The same is true when a branch is merged back into trunk; there isn't a clear historical description of what has already been integrated into trunk. (NOTE: svn has put some effort into improving their branching and merging capabilities, but they are still not up-to-par with DVCS tools)

...