This repository has been archived by the owner on Nov 2, 2020. It is now read-only.
Git workflow proposal #3
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| --- | ||
| title: Git Workflow | ||
| author: David Davis | ||
| created: 01-Apr-2017 | ||
| status: Accepted | ||
| --- | ||
|
|
||
| ## Summary | ||
|
|
||
| This PUP is to redefine our git workflow to use cherry-picking instead of | ||
| merging commits forward from x.y-dev branches. Using this model, Pull Requests | ||
| (PRs) would be made against master and then bug fixes would be cherry-picked to | ||
| the older x.y-dev branch as needed. Hotfixes would be cherry-picked from | ||
| x.y-release branch into the x.y-dev branch and master. | ||
|
|
||
| ## Motivation | ||
|
|
||
| Currently in Pulp, we merge features to master. However, bug fixes are merged | ||
| to x.y-dev branches which then are merged to master (a process called "merging | ||
| forward"). More info about merging forward can be found in our documentation: | ||
|
|
||
| http://docs.pulpproject.org/en/2.12/dev-guide/contributing/merging.html#merging-your-changes | ||
|
|
||
| Also, a good illustration of this process is outlined in the following webpage: | ||
|
|
||
| http://nvie.com/posts/a-successful-git-branching-model/ | ||
|
|
||
| As a result, our git history is complex with multiple branches feeding into one | ||
| another and numerous merge commits. Using cherry-picks would simplify our git | ||
| history by keeping it more linear and eliminating merge commits. | ||
|
|
||
| Opening a PR requires knowledge of our current process. This can create a | ||
| barrier to entry for community members. With the cherry-pick model, all PRs are | ||
| opened against master and the onus to update older x.y-dev branches is on the | ||
| person merging (who usually has better knowledge of which branches a commit | ||
| needs to be applied to). | ||
|
|
||
| We've also seen some mistakes where branches are accidentally merged into other | ||
| branches. These situations have occurred multiple times and each time required | ||
| multiple people working several hours to resolve the problem. After being | ||
| resolved, the git history is convoluted with huge negative commits which later | ||
| caused confusion at release time. Commits could be accidentally cherry-picked, | ||
| but this would be less disastrous since it's only a single commit. | ||
|
|
||
| Changes that we want to only be made to older x.y-dev branches (but not to | ||
| newer ones) still need to be merged forward. We could elminate the need for | ||
| this with cherry-picking. It's worth noting though that this rarely happens. | ||
|
|
||
| In our current workflow, all bug fixes are made against x.y-dev branches and | ||
| merged forward. If they were instead cherry-picked from master to x.y-dev | ||
| we have the choice of backporting with conflict resolution or leave the changes | ||
| exclusively on master. | ||
|
|
||
| Moreover, when the merge forward happens, sometimes the implementation needs to | ||
| be reworked and conflict resolution done. This reworking never goes through a | ||
| review process and is usually done by a single developer who tries to get it | ||
| right. This has caused issues in the past when a merge forward left the | ||
| implementation on master broken. | ||
|
|
||
| ## Detailed Design | ||
|
|
||
| This proposal would keep the current branching scheme[1] involving master, | ||
| x.y-dev branches, and x.y-release branches. The workflow by which changes are | ||
| moved between the branches would however change. The cherry picking process will | ||
| apply to Pulp2, Pulp3, and future Pulp versions. | ||
|
|
||
| The process works a bit differently depending on whether the change is a new | ||
| feature, bug fix, or hotfix. | ||
|
|
||
| #### New Feature | ||
|
|
||
| 1. All feature PRs are opened and merged against master. No cherry picking will | ||
| occur since new features only land on master. | ||
|
|
||
| #### Bug Fix | ||
|
|
||
| 1. All bug fix PRs are opened against master | ||
| 2. After the PR is ready to be merged, the person merging the commit would | ||
| first merge the commit to master. Then, this person cherry-picks this change | ||
| from master to the latest x.y-dev branch by opening a PR with the change. | ||
| 3. The person cherry-picking would also need to track in Redmine which dev | ||
| branch(es) the change has been applied to. | ||
|
|
||
| #### Hotfix | ||
|
|
||
| The current hotfix process[2] would remain basically unchanged. However, | ||
| instead of merging forward the commit, we'd use cherry-picking. | ||
|
|
||
| 1. The affected release would be branched to create the hotfix branch and a | ||
| hotfix would be applied to the hotfix branch. | ||
| 2. After the release, the hotfix gets cherry-picked via PR into the x.y-release | ||
| branch. | ||
| 3. The change is then cherry-picked into the x.y-dev branch and master by PR. | ||
| 4. The person cherry-picking would also need to track in Redmine which release | ||
| branches the change has been made to. | ||
|
|
||
| ### Conflicts | ||
|
|
||
| If the cherry-pick has conflicts, we should abandon the cherry-pick and | ||
| encourage users to upgrade to the next Y release. | ||
|
|
||
| ### Using git cherry-pick | ||
|
|
||
| When performing the cherry-pick in git, use the `-x` flag which adds the | ||
| original commit id to the commit message of the cherry-pick. This will help | ||
| track changes across branches. | ||
|
|
||
| ### Updating Redmine | ||
|
|
||
| When a cherry-pick is merged to a dev branch, the release field of the issue in | ||
| Redmine should be set to x.y.next. For example, when a cherry-pick is merged to | ||
| 2.13-dev, the issue's release field should be set to 2.13.next. Then when the | ||
| change is released, its field will be updated with the actual value (e.g. | ||
| 2.13.4). For a hotfix that is applied to a release branch, the release branch is | ||
| set to the hotfix release version. | ||
|
|
||
| ### Timeframe | ||
|
|
||
| When this proposal is accepted, we'll work with release engineering to determine | ||
| a date to switch to cherry-picking. After a date has been picked, an | ||
| announcement will be sent out to the pulp-dev list with the planned date. After | ||
| this date, we should begin to follow this proposal. Also, we'll enter in Redmine | ||
| issues to update our docs accordingly after this proposal has been accepted. | ||
|
|
||
| ## Drawbacks | ||
|
|
||
| Git is really built for changes to be merged between branches. For example, | ||
| when cherry-picking a change, the commit is assigned a new id but when merging | ||
| the commit, it retains the same id. It makes looking up the commit to see what | ||
| branches it's been applied to easier. If we cherry-pick, we have to track this | ||
| information externally to git. This means also having to remember to update | ||
| Redmine. | ||
|
|
||
| Switching to this model also creates some inconsistency. Looking at git history, | ||
| you will see merge commits between branches (e.g. merges from the x.y-dev branch | ||
| to master) up until the date in which we switch to cherry-picking. | ||
|
|
||
| Defaulting to master, and requiring extra steps to get a change onto the | ||
| current x.y-dev branch, is likely to result in fewer bug fixes making it onto | ||
| x.y-dev branches. We can and should try to be diligent about cherry-picking fixes | ||
| to the x.y-dev branch, but the path of least resistance may prove too tempting | ||
| at times. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| ### Keeping the current model | ||
|
|
||
| See the prior sections like "Motivation" and "Drawbacks" for a discussion on this. | ||
|
|
||
| ### Merge forward less often | ||
|
|
||
| Perhaps keep the current model but merge less often. This would help to prevent | ||
| accidents and keep the git history cleaner. There are some drawbacks however to | ||
| this. One would be an increased chance of merge conflicts if we merge less | ||
| often. However, we could also use Github's PR functionality to allow for code | ||
| reviews of merges. This would also help to decrease accidental merges. | ||
|
|
||
| ## Footnotes | ||
|
|
||
| 1. https://docs.pulpproject.org/en/2.12/dev-guide/contributing/branching.html | ||
| 2. https://docs.pulpproject.org/en/2.12/dev-guide/contributing/branching.html#hotfix | ||
|
|
||
| [1]: https://docs.pulpproject.org/en/2.12/dev-guide/contributing/branching.html | ||
| [2]: https://docs.pulpproject.org/en/2.12/dev-guide/contributing/branching.html#hotfix | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who exactly is "release engineering" in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In regards to updating our packaging stuff, etc., I'm happy to do the legwork to get the release engineering stuff ready for us to use cherry-picking although I'm a bit unfamiliar with our release process. I might ask for a volunteer first though. I did talk to @pcreech and he said he would approve and have the final word on whether or not we're ready for us to switch to cherry-picking (assuming this PUP is accepted).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes working with @pcreech is great. He can set the timeline to ensure the release process will not have issues if this switch is made. Thanks for coordinating.