Opentrons: RFC: Implement Gitflow as our branching/release process

[mcous]

overview

This is a RFC for our current development process as it relates to branching, tagging, and ~releasing~ generating release artifacts. Specifically, it recommends adopting the Gitflow workflow. Everything old is new again.

The Gitflow workflow is a slight extension to our current feature and release branch workflow that adds a few more branches to the mix to improve our release [artifact generation] process

requirements

With whatever workflow we implement, our goals are:

• Releases and release testing do not block merging work for that leaf project
• Releases and release testing for one leaf project do not block anything (merges / releases) for another leaf project
• Some sort of changelog generation should happen

Where our "leaf projects" right now are:

• App + API (Opentrons system)
• Protocol Designer
• Labware Library + Creator

proposal

This RFC proposes that we implement a basic Gitflow branch structure, where:

1. All development happens on a branch called edge
2. All feature / non-urgent fix work happens on feature branches from / into edge
   a. Feature branches are squash-merged into edge
3. When it's time to build a release artifact, a release branch is cut from edge
   a. Versions are bumped (while we're still doing filesystem based versioning)
   b. Changelogs are generated
   c. Release QA performed
   d. Hotfixes are added to the release branch as necessary and release branch is rebased accordingly to ensure the Git history matches the changelogs
4. When QA is done, release branch is regular merged into master
5. That commit is tagged and release artifact is built
6. master is regular merged into edge
7. If an urgent fix is needed
   a. A hotfix branch is branched from master
   b. Hotfix is bumped, changeloged, and QA'd just like a release branch
   c. hotfix branch is regular merged into master
   d. master is tagged
   e. master is regular merged into edge

Read the Atlassian article above for a more in-depth discussion and diagrams. Please note that we don't necessarily need to treat the Atlassian (or any other) article as gospel; we're just looking to add a branch or two to make our releases a little more sane.

Things to note:

• Steps 1-3 are very close to what we've already been doing, except that:
  
  
  
  • We have tagged on edge and/or the release branch in the past
  
  
• This flow is most applicable to the App/API project; see below

pros / cons

Pros

• Not _that_ different from what we did on the 3.10.x line
• Keeping all our tags on master means we get isolated release _and_ changelog generation tools don't get confused
• QA + release does not block feature work on edge
• Hotfixes also do not block feature work on edge

Cons

• More branches
• Different merge types depending on target branch
• Will have to be a manual process to start; manual processes can be tough and error prone

leaf projects that aren't App/API

Our other leaf projects (PD, LL+C) are web projects, and they have the following concerns:

• They are versioned independently from App/API
• Releasing the exact artifacts that get QA'd is cool

For these projects, this RFC proposes we select one of these two options:

1. Keep doing basically what we're doing:

   1. Feature/fix branches for regular work
   2. Cut a release branch off edge
   3. Bump + tag release branch, QA
      
      
      
      1. If bugs found, push fixes to release branch, new bump + tag, QA
      
      
      2. release branch will need git history modifications after fixes to keep history clean
      
      
   4. Regular merge into edge

2. Implement a full Gitflow for each leaf project, where each leaf project has its own master branch (e.g. protocol-designer-master)

Pros to option (1):

• Close to what we're already doing
• We deploy "exactly" what we QA to production
  
  
  
  • With option (2), we might not be able to QA the tagged build, but instead be satisfied that there are no code changes between the QA'd release branch and the tagged master build
  
  

Pros to option (2):

• Keeps release processes the same across projects
• We have a branch to point users to if they're curious about the source code of the current release

future workflow changes / additions

• This will likely have to start as a fairly manual process until we can build automation
  
  
  
  • Process involves different types of merges depending on the target branch
  
  
  • We're going to have to be careful and/or change GH settings when we're doing releases
  
  
  • We'll probably mess things up once or twice
  
  
• Automation of some sort using a privileged bot user that's the only thing that's allowed to push directly to master and edge will be (more than likely) necessary
  
  
  
  • Frameworks exist for this sort of thing
  
  
  • e.g. probot
  
  
• Tag-based versioning of leaf projects is still a goal, but I don't see any way Gitflow blocks that

---

Edit 1: s/develop/edge per @sfoster1's suggestion
Edit 2: Tried to make it more clear that we're talking about our release artifact generation mechanisms, not our artifact deploy mechanisms

[sfoster1]

For our purposes we can probably s/develop/edge/, there's no need to change the name of the branch.

[Kadee80]

👍

[IanLondon]

Releases and release testing do not block merging work for that leaf project
Releases and release testing for one leaf project do not block anything (merges / releases) for another leaf project

Can we do this by separating artifact deploys from code merges, instead of making more code merges that mean different things?

[mcous]

Can we do this by separating artifact deploys from code merges, instead of making more code merges that mean different things?

@IanLondon Sorry this wasn't clear in the description, but this RFC is not meant to affect or dictate anything about artifact deploys. Artifact generation being tied to artifact deploy in the RA/API is a separate issue, but is also not the reason that merges are blocked. This RFC is concerned with artifact __generation__.

We have tried a few strategies with in terms of artifact generation:

1. Generate artifacts from bump on edge. Block merges into edge during QA
   
   
   
   • This is bad for everyone
   
   
2. Generate artifacts from bump on edge, do not block merges into edge during QA
   
   
   
   • Works great if QA is all green
     
     
     
     
     
     • If QA has failures, we must merge fixes into edge, by necessity picking up all other merges into edge into our release (potential new bugs and all)
     
     
     
     • This can increase the scope of the further QA because of those other merges
     
     
     
   
   
3. Generate artifacts from release branch, merge fixes into release branch, do not block edge
   
   
   
   • Did this for the 3.10.x line
   
   
   • This works pretty well! If any QA tests fail, specific fixes can be patched into the release branch without picking up changes from edge
   
   
   • However, it can be complicated to get those fixes back into edge
   
   
   • Also, we end up with release tags on commits on orphaned branches, which confuses tooling like git describe
   
   

So, this RFC is about extending option (3) to:

• Have a clear way to get QA fixes and hotfixes back into the mainline development branch
• Ensure tags point to commits that the main development branch knows about

[IanLondon]

For the artifact generation approaches 2 and 3 above, I think they're trying to solve this: It can be a problem if we QA candidate No. 1, find some problems, merge some fixes, and in the mean time get "outside" changes from other PRs getting merged that don't directly address the project we're QA'ing and might mess it up. When we have candidate No. 2 ready to QA, we ideally want to only test the things that failed in the previous QA round. But since other things snuck in, we can't trust that things that passed will still pass, so we have to start QA from scratch.

One way out, number 3, is to isolate QA candidates so that they don't pick up "outside" changes, and git flow is a clear way to enable that isolation.

Isolating QA candidates speeds up QA of those projects. But if we isolate our release branches, it's likely that once in a while we'll have a situation where QA for a project passes, then the release branch gets merged back in to edge. As a result of the merge, that project no longer works right. That's a bad place to be in, especially considering that we wouldn't be QA'ing the merged-into-edge code, we are QA'ing it only before merging. We could release a dud even after it passed QA.

I'd rather take the route of not isolating QA candidates from the commits in edge. Generate an artifact every commit into edge, do QA on one of those artifacts, tag it if it passes. (This is pretty much what we're doing now for PD/LC, it's strategy 2 on the post above)

• Pro: confidence that our release is exactly the artifact that we tested.
• Con: that means more repeating QA tasks if a QA round fails.

Hopefully we can make QA less time-consuming by automating more of it as we go. That would lower the con side here. IMO I'd rather we invest energy into making QA quicker and cheaper for us than to invest energy into developing tooling/automation that would make the gitflow multi-project workflow less error-prone.

I def might be missing something here! I am pretty biased b/c I'm not dealing directly with Run App / API release problems. For the web projects, the gitflow approach seems to me like it's making everything more complicated without any benefit.

[sfoster1]

@IanLondon Your proposed flow does require merge freezes during the timespan of someone declaring work on a pr to bump the version and generate changelogs and that pr getting merged, so we don't miss stuff in the changelog. That can actually stretch for a while between waiting for ci and making sure everybody gets eyes on it.

I also disagree with the sentiment of "let's just make our QA better". You're right, we should. But we also have a lot going on, and the key ways in which we can improve qa are not small tasks in and of themselves - end to end testing, browser automation, that sort of thing - so we're realistically not going to get to them for a while. In the meantime, we shouldn't just do nothing.

Maybe we should split this on a per-leaf-project basis? It makes sense to do this workflow for runapp/robot software, but it's separate from the pd and ll process to such an extent we already proposed having separate master branches. Once we're there we might as well just separate the processes entirely. PD and LL keep doing their artifact promotion strategy, runapp/robot software (gotta think of a punchier name here) do this kind of gitflow thing. They should work together just fine.

[mcous]

But if we isolate our release branches, it's likely that once in a while we'll have a situation where QA for a project passes, then the release branch gets merged back in to edge. As a result of the merge, that project no longer works right.

(Emphasis mine)

@IanLondon I _believe_ this is a misunderstanding of the flow. The process is as follows:

1. Release branch cut from edge
2. Release branch goes through QA, maybe some commits are added
3. Release branch merges into master (or proctocol-designer-master or whatever)
   
   
   
   • Prior to this merge, the contents of master are exactly the previous release
   
   
4. master is tagged
   
   
   
   • Presumably this generates the artifact, but we could choose to use the artifact from the release branch if we wanted
   
   
5. master is merged into edge

@sfoster1 can correct me if I'm wrong here, but I was under the impression that the order of the merges is such that the contents of master must match the contents of the release branch exactly after the release > master merge. Then, if we choose to generate an artifact from master, it's the same source code as the build that just got QA'd). If that were not the case, then yes, the value of this flow would be dubious.

For the web projects, the gitflow approach seems to me like it's making everything more complicated without any benefit.

What's been the process thus far for a PD hotfix that needs a fix applied directly because of back-compat risks with changes to edge? Without gitflow, I'd imagine that based on the current process it would have to be something like:

1. Create branch from the last release tag
2. Make changes, commit, tag that hotfix branch after QA (?)
3. Merge back into edge

---

As I'm working through this in my head, I'm starting to think that (no matter what we do) we're going to be forced to move the CHANGELOGs into GH releases instead of in-source (the tooling exists to do this). No matter what non-blocking flow we're doing, we're going to end up with weirdness about tags, commits, and ordering of log entries.

An advantage of Gitflow for __all__ or projects in this context is that, if we only ever tag master, we can then generate _all_ of our change logs from master. Because of Gitflow, our logs will then always be accurate. (Or rather, our logs will always contain the commits we are releasing. Logs will be accurate if we committed all our stuff properly).

[sfoster1]

I am ok with putting changelogs in to GH releases. We'd presumably be publishing alpha releases there also, right? Anything tagged?

And yeah your read on the master merge is correct, that should always be fast-forward.

[mcous]

We'd presumably be publishing alpha releases there also, right? Anything tagged?

Yup, and they'd be marked as prerelease

[IanLondon]

Re @sfoster1's comment about merge freezes for version bump + changelog:

IMO doing a bump to application version after QA is undesirable, b/c if the application is aware of its own version, it's going to behave differently after the bump, in ways that the pre-bump QA couldn't cover. I'm not aware of any version-self-aware behavior in RA/robot, but PD uses its own version to show update messages (not changelog) and to run migrations, so PD needs its version bumped before QA happens, and generally as soon as there is a PR that uses that version for a new behavior.

Have we talked about about moving away from committing the changelogs (keep them in a git tag message or something)? Edit: mentioned above

These are not trivial moves but they might be easier than moving to gitflow. If we don't need this extra process of PR'ing and merging a version bump + changelogs, then strategy 2 is non-gating and we don't have to build/find/cobble the tooling and automation for doing multi-project gitflow safely.

We'd still have the issue where QA may need to be repeated from scratch when commits came into edge and the first QA failed. I don't think we should do nothing, I think that now is the right time to begin incrementally automating QA and prioritizing that. If speedier QA is the main pain point which isolating release branches from edge is intended to solve, our time is better spent making QA better now instead of restructuring our git workflow to limit what code goes into releases. Gitflow isn't an inexpensive or fast solution, and I don't think it would be a long-term one.

---

Thanks for the clarification @mcous. As long as it's only fast-forwards to master, my reservation about releasing a thing we didn't QA isn't a concern. Thanks!

So just to rehash to make sure I understand: if we do gitflow with per-deploy-set master branches, we're tagging only per-deploy-set master branches. I'm calling them "deploy sets" because multiple projects: RA+robot, and LL+LC are deployed together. There's no real master branch (we can call the run app / robot deploy set master if we want but it's not authoritative over, or more complete than protocol-designer-master or labware-master or any other master branches). Is that right?

@mcous re hotfixes: correct, that's been the recent hotfix flow for PD, though sometimes the bugfix PR to edge was made before we decided it was bad enough to do a hotfix release, or no PR to edge was necessary because in edge the problem is no longer relevant (fast-changing feature, I think this happened with the deck map).

---

We did gitflow a while back, by the book with no special twists, and we abandoned it. Gitflow works against GitHub: it requires us to remove GitHub branch permissions and types of merges allowed, which is dangerous (https://github.com/petervanderdoes/gitflow-avh/issues/358); it requires us to sometimes use the git flow CLI instead of the GH merge button. It requires us to regularly push commits without going thru PR reviews (to keep branches in sync), which is fine until somebody makes a mistake when resolving a merge conflict. Gitflow requires us to spend more time fixing merge conflicts, cleaning the history, thinking about git flow diagrams, and thinking about when to squash vs normal merge. Doing gitflow for multiple "deploy sets" (RA+Robot, LC+L, PD) adds an additional dimension of complexity. Since release branches are isolated from edge, edge is prone to getting a big dump of changes after a release goes out, which might mess up other work in-flight and is anti-continuous-integration. Even after we all get over the initial learning curve, gitflow would require us to each take more care and thought day-to-day to accomplish git/GH tasks.

[mcous]

Thanks everyone for your thoughtful feedback! I've talked to everyone in person, and here's what we're going to move forward with:

• Leave release branch process unchanged for web projects (PD, LLC)
  
  
  
  • These projects are relatively isolated
  
  
  • The risks that a gitflow-like branching structure is designed to mitigate don't come into play as much
  
  
  • @IanLondon to codify current web-projects process in a docs PR to come
  
  
• 3.11.0 will be force pushed to master and tagged there once it has passed QA
  
  
  
  • From this point forward, app/API releases will be on a gitflow like branch structure
  
  
  • To sum up, we're adding a master branch to collect releases on top of our existing release QA branch process
  
  
• @sfoster1 or myself will need to open a PR to update/add this release process to contributing documentation
• Nobody needs to change how they're working on a day-to-day basis unless your are actively working to release a new version of the app/API
  
  
  
  • The git flow CLI is not required and nobody should need to install it
  
  
• Someone (probably me) will explore moving change logs to GH releases to make our lives easier

3.12.0 will likely involve an amount of manual merging (and temporary removal of branch protections to do so). Automation of this process should be explored ASAP; maybe we can even beat 3.12.0

Marking this RFC accepted; will close when documentation is updated and 3.11.0 is on master

[mcous]

We are using this process to release the app as of 3.11.x