OcsSourceCodeManagement
This document describes how we manage OCS source code in the HLSW Group, including how we structure our repository, policies for introducing changes to the codebase, how releases will be organized, etc. At the heart of our system lies git, a popular distributed version control system. Git is far more powerful than our old revision control system, svn, and with that power comes increased complexity and new options for collaboration. Some conventions and structure must be imposed in order to prevent branching and release chaos.
Most of the ideas presented here are taken from “A successful Git branching model” and the presentation in https://www.atlassian.com/git/workflows with a few adjustments to match our particular situation. I suggest reading these pages before continuing. Since I know you won't read them (hey we're all busy), I'll try to include some of the explanation in this document.
In summary, we are using “gitflow” but each developer works with a fork of the central repository. Changes are made in “feature” and “hotfix” branches and merged in via pull requests after an informal peer review process. The two main differences between our setup and a typical open source project are that we’re all “project maintainers” and we’ll need two “master” branches as we’ll explain in the following sections.
We have a GitHub organization called gemini-hlsw with an ocs repository. In this document I’ll refer to this repository as our “central” repository, though of course this is just a convention that we’ll follow. The ocs repository currently houses the bulk of all high-level software (with a few notable exceptions like ITAC, LCH, etc.). Eventually this will be split into multiple product-focused repositories like ocs-model, ot, pit, etc. In the meantime the combined ocs repository is used for developing the PIT, OT, ODB, and QPT and releases are made from it.
Rather than pushing directly to gemini-hlsw/ocs, each developer should fork this repository in his own personal GitHub account. Though you have write access to the central repository, the idea is that you spend most of your time working with your fork. This keeps the central repository free of clutter and failed experiments, etc. Changes should be merged in via pull requests much as for an open source project. Instructions are provided for Setting Up Your Workspace for OCS Development.
We follow a gitflow-inspired branch convention with shared master, develop, and release branches. These branches should be considered almost read-only in that they are usually updated via pull requests from maintenance branches. Integration branches essentially live in the central repository and are copied and updated to each developer's fork. Developers create and work on maintenance branches and merge their efforts back into the central integration branches.
If you're new to gitflow, think of develop as the SVN trunk. It contains the current snapshot of integrated development across the team. The master branch contains just the release history and the release branches are temporary branches created to fix last-minute bugs before a release.
Note that our situation is complicated by the fact that the PIT is on a different release schedule than the rest of the OCS software. For that reason, we’ll have a master for PIT, cleverly named pit-master, and a master for the rest of OCS, ocs-master. Both are updated from the release branches as in standard gitflow. At some point, the ocs repo will be split into finer grained project repositories and this deviation will disappear.
Release branches exist for the sole purpose of integrating bug fixes to a pending release. They are created from the current state of the develop branch shortly before the release, ideally when all features have already been incorporated into development. They are ultimately merged into the appropriate master branch when the release is made. Changes they contain must also be merged back into the mainline develop branch.
We will use the following naming convention for release branches:
release/PRODUCT-VERSION
where the PRODUCT is currently either ocs or pit. From the ocs release comes the OT/ODB/QPT but the PIT is separated because its release cycle is out of phase with the rest of our offerings. For example, the 2015B.1.1.1 release of all non-PIT software is named
release/ocs-2015B.1.1.1
Maintenance branches are created in each developer's fork of the central repository. They should not be directly pushed to the central repository but rather merged into the appropriate integration branch via pull requests.
We will use three different flavors of maintenance branches: feature, bugfix, and hotfix. They all share the fact that they are created by individual developers from the shared integration branches and that they are merged back into integration branches via pull requests. They differ in their naming convention, objective, review policy, and the particular branches from which they are created and to which they are merged.
We’ll lump together non-urgent bug fixes and new features into one “features” category. Basically any change that can be scheduled for a future planned release before the release branch is created (as opposed to a patch to an existing release) should be done in a feature branch that is based on and merged back into develop. Ideally all feature branches will be related to JIRA issues and therefore should reference the corresponding issue in the name.
| Naming Convention | feature/ISSUE For example: feature/REL-123 |
| Branched From | develop |
| Merged Into | develop |
| Review Policy | At least one other developer should comment on the pull request before it is merged. Try to get feedback from the most appropriate developer, if any. |
Bugfixes are reserved for bugs in release candidates during the pre-release testing cycle. They are branched from the corresponding release branch and merged back into the release branch via a pull request.
| Naming Convention | bugfix/ISSUE For example: bugfix/REL-123 |
| Branched From | Corresponding release branch. |
| Merged Into | Corresponding release branch (and possibly ongoing develop if critical). |
| Review Policy | Because bug fixes will be applied to soon-to-be-released software, at least a couple of developers should review and approve the pull request. |
These are bug fixes to already installed production code that cannot wait for the usual release cycle.
| Naming Convention | hotfix/ISSUE For example: hotfix/FR123 |
| Branched From | Corresponding master. For example, for an ODB issue this would be ocs-master. |
| Merged Into | Corresponding master and ongoing develop. |
| Review Policy | Because there is good deal of risk involved with updating the production release without going through the usual month-long release testing cycle, the high-level software lead (or a delegate if not available) must review and approve of all hotfix pull requests. A second developer should also sanity check the change before the pull request can be merged. |
To make it easier to merge your feature or bug fix back to the appropriate integration branch, and to simplify the pull request/peer review process, please use the git rebase command rather than git merge when you must incorporate changes made by others on the original integration branch. Rebasing will make it appear as if you did all your work after the latest changes to branch from which it was created.
What if you've already pushed changes to your public fork of the repository? Won't that change the history out from under other developers who are using your fork? Yes, use rebase anyway. The fork of your repository is your work area. If you're collaborating with someone on the team they will have to deal with the rebase. If someone outside of the team is interested in using our code, they should be forking the central repository anyway. See the next section for a look at the git commands involved in this process.
When a pull request is first opened, you should first be up-to-date on the integration branch from which your maintenance branch is created. For example, when starting a new feature you'd update your copy of develop:
git checkout develop # switch to local develop branch
git pull central # merge in changes from the central repository
This assumes you've first named the central repository central with
git remote add central https://github.com/gemini-hlsw/ocs.git
Now your local clone of the OCS repository has been updated from the central repository but these updates are not yet included in your fork on GitHub. If you want to do that, just push the changes there:
git push origin
You're ready to begin working on your feature so you create and switch to the new branch and make some changes.
git checkout -b feature/REL-123
# edit something
git commit -a -m "REL-123: foo"
# edit something else
git commit -a -m "REL-123: bar"
Commit these changes to your fork for safekeeping or to start a new pull request.
git push origin feature/REL-123
Eventually others will merge in their pull requests and you'll no longer be up to date with the central repository. You'll want to make sure that your changes are still compatible with the latest changes from other developers. We recommend using the rebase command for this. It will change your commits to make them as if based on the tip of the develop branch.
git checkout develop # go back to develop branch
git pull central develop # incorporate the latest updates
git push origin # push them to your fork
git checkout feature/REL-123 # switch back to your feature branch
git rebase develop
The rebase command will base your changes from the current tip of the develop branch. (If you've already pushed commits to your fork, see the note about force pushes.) At this point you can fix any merge issues that might come up, run tests, continue developing or initiate your pull request merge. When you're ready, send the latest updates to your fork and then open the pull request on GitHub.
git push origin # publish your work for review
Keep in mind that you can make changes even after you open the pull request. Any updates to your branch will be reflected in the pull request. Once reviewed, you can merge your updates back into the central develop integration branch with the green GitHub Merge button assuming there are no conflicts. At that point there's no need to keep your feature branch around.
git push origin :feature/REL-123 # delete the remote public branch
git checkout develop # switch to the develop branch
git pull central develop # pick up the merged-in changes in develop
git branch -d feature/REL-123 # delete the local branch
git push origin develop # update your fork
Assuming you've already pushed changes to your fork, a rebased local feature branch can no longer be pushed cleanly.
$ git push origin feature/REL-123
To https://github.com/swalker2m/ocs
! [rejected] feature/REL-123 -> feature/REL-123 (non-fast-forward)
error: failed to push some refs to 'https://github.com/swalker2m/ocs'
You can get around this with a push -f or push --force
$ git push -f origin feature/REL-123
...
+ 7d1b7de...d02a552 feature/REL-123 -> feature/REL-123 (forced update)
This will keep the "network" graph commit/history clean when the PR is ultimately merged.
Maintenance branch changes are merged into the respective integration branches via pull requests on GitHub. Nobody will be exempt from the pull request rule and nobody should feel intimidated by peer reviews. There are several advantages to adopting pull requests and an informal peer review process:
- Exposes everyone to parts of the codebase with which they might not otherwise be familiar. This increases awareness of how things work across the team and reduces "islands" of knowledge.
- Provides an opportunity to learn from one another.
- Others are likely to have alternative suggestions or better ways to approach problems that we wouldn't have considered alone.
- You might not be as tempted to take short-cuts if you know others will be looking at your work.
- Particularly for hotfixes and bugfixes, there is a chance someone else might catch mistakes that you overlooked.
Everyone should be notified of new pull requests and should use that as an opportunity to contribute to the discussion. This is an informal review process but particularly for hotfix and bugfix pull requests, we require approval from other developers before allowing the changes to be merged.
Note that there's no reason to wait until you've finished a feature in order to open the pull request. You also can generate the request as soon as you've created the corresponding branch and pushed it to your fork whether you've done any work yet. A pull request can also be opened when you just want to ask a question or start a discussion of alternatives. Changes to your branch will appear in the pull request automatically so you won't have to delete it and redo it later as you progress.
Make sure that your commit messages reference the corresponding JIRA issue if there is one. If there isn't one, whatever you're doing should be sufficiently trivial that it requires no more explanation than a short phrase and there should be no need to let the science staff know about the change. One example of this kind of change would be cleaning up source code, marking Java variables as final, fixing typos in comments, etc. If your changes are more involved, take the time to create a JIRA issue and assign it to the proper release so it can easily be included in the release notes.
JIRA has been configured to show commits to our repository if you're viewing an issue with commits that reference it.
The master branch should always be behind develop and should merge cleanly at the end of the semester. Unfortunately that may not always be the case. Basically we want to do whatever edits are necessary to the master branch to make it match the release branch (or develop branch if no release branch was created). There is a merge strategy called "ours" that resolves all issues with "our" version. Unfortunately there isn't a "theirs" strategy which is what we need here to take the master to the current state of develop. So if you find conflicts trying to merge the semester's changes, you can effectively achieve a merge with the develop or release branch state as if git merge -s theirs existed with the following sequence:
git checkout ocs-master # for example, for the ocs-master branch
git log # and take note of the last commit reference, say e93f275
# Advance ocs-master without changing its content.
git merge -s ours develop # or the release branch if there is one
# Change working tree and index to desired content.
# --detach ensures develop will not move when doing the reset in the next step
git checkout --detach develop
# Move HEAD to ocs-master without changing contents of the working tree and index.
git reset --soft ocs-master
# attach HEAD to ocs-master
# This ensures ocs-master will move when doing 'commit --amend'.
git checkout ocs-master
# Change content of merge commit to current index (i.e. content of develop)
git commit --amend -C HEAD
At this point we have ocs-master with the series of commits that were performed since the previous release. Since that can be 100s of commits that we don't want to see again in ocs-master you can squash them all into one big release commit with
git reset --soft e93f275 && git commit
where e93f275 is the reference to the previous release commit. At this point you can push the change to central and then tag it with:
git push central ocs-master
git tag -a 2015B.1.1.1 # for example, for release 2015B.1.1.1
git push central 2015B.1.1.1
There may be instances where you want to check out a branch remotely from another user's repository, for example to work collaboratively on a branch or to create a branch off one of their branches to make changes dependent on their branch for a larger feature.
In order to do so, you must generate an SSH key and add it to your github account. Detailed instructions to do so and to verify that everything is working properly can be found at https://help.github.com/articles/generating-an-ssh-key.
In order to access another user's github branches, you must remote-add that user and then fetch their list of branches. For example, to set up access to and retrieve swalker2m's list of OCS branches, run the following commands:
git remote add swalker2m [email protected]:swalker2m/ocs.git
git fetch swalker2m
Then another user's branch can be checked out into your own repository as follows:
git checkout -b feature/REL-123 swalker2m/feature/REL-123
You can make changes on this branch, or branch on this branch, and then create PRs against their branch, which should only be visible to you and the owner of the original branch.
If you receive a message about permissions being denied (public key), instructions to investigate and fix this problem can be found at https://help.github.com/articles/error-permission-denied-publickey.