Details
-
Type:
RFC
-
Status: Implemented
-
Resolution: Done
-
Component/s: DM
-
Labels:None
-
Location:This issue
Description
There is a DM Branching Policy page on Confluence, but it says that it is ancient. Some policies around git branches are listed on the Using Git for LSST Development page, but they're not very specific.
A de facto policy was created by gitolite push restrictions that limited branch names and, to some extent, usage. These restrictions no longer apply on GitHub.
Some developers have felt a tension between maintaining their time-ordered development history and a cleaned-up, reviewable, one-commit-per-feature history within a single branch.
I propose the following to replace the appropriate sections of the above Confluence pages:
- master remains the main integration branch and must always be deployable. No development occurs directly on master in any lsst (DM team) or lsst-dm repository, with the exceptions of documentation and very small fixes.
- The next branch and release branches are created as necessary for integration.
- tickets/DM-NNNN branches are the "official" history of development. All reviews happen on these branches, and all merges to master, next, or a release branch happen from them. These branches should be rebased with cleaned-up (squashed/separated, correctly-commented) history.
- u/username/anything branches are available for developers to do whatever they want. They can start investigative work on a feature without filing a JIRA ticket. They can keep as-developed history separate from the official history. Merges from such branches should not occur, but a tickets/DM-NNNN branch can be created at the tip of such a branch in order to merge it.
- Pull requests from other repos including personal repos are the equivalent of ticket branches. They need to have a DM-NNNN JIRA issue identifier and follow all the policies of in-repo ticket branches.
Attachments
Issue Links
- relates to
-
- Done
Activity
In a similar spirit, is the item
PR reviews should generally use the "Files changed" tab in GitHub rather than the "Commits" tab in case the branch is rebased as a result of the review.
due to GitHub "losing" the comments on rebasing? If so, could this be re-written to allow for commenting under the "Commits" tab IFF GitHub fixes this behaviour?
Paul Price
added a comment - In a similar spirit, is the item
PR reviews should generally use the "Files changed" tab in GitHub rather than the "Commits" tab in case the branch is rebased as a result of the review.
due to GitHub "losing" the comments on rebasing? If so, could this be re-written to allow for commenting under the "Commits" tab IFF GitHub fixes this behaviour? "Branch should be deleted" – I was a little wishy-washy. I do think that it will be advisable to do so, but I worry a little that we don't have all the tools to track things down yet, making it easier to find things if the branches are left around.
The "by LSST-paid" was intended to apply to the entire penultimate clause. This means that non-LSST code does not need a JIRA ticket. I can live with that.
I don't think GitHub can avoid "losing" comments on rebasing; the original commits to which the comments are referenced can be garbage-collected at any time since nothing is pointing to them. Perhaps they could double-link the comments to the commits as well as the PR, but regenerating the diffs would require some way of preserving the commits as well. I'd rather have a clear policy for now and change it if the tools are improved later than try to come up with a policy that will hold for all time.
I am personally in favor of keeping ticket branches, lat least for a few years. I have found them very helpful on the HSC side when cherry-picking commits. I agree that the names do cause clutter, though at least github has a very good search mechanism for branches.
I am not comfortable assuming that users can remember to include the ticket branch in the commit message when merging to master, and once a merge to master has occurred it is hard to fix the commit message.
Proposal as written with these changes:
- Release candidate branches if necessary instead of next
- Develop automated process to turn ticket branches into tags (at last commit before merge) with name different from tickets/.
I like it. Minor requests for clarification:
Is that really a requirement, or is it really "may be deleted"? If the former, will there be an effort to clean up old ticket branches?
Thanks!