Re-evaluate git flow approach?

[abuiles]

Have we considered a different git flow (do not confuse with GitFlow) on the go monorepo? The reason why I'm bringing this is that our current approach of release-service-name-vx.x.x can be kind of confusing specially for outside contributors.

I'm bringing this because of #1953 (review) -- he saw the issue and just branched off of master to submit a fix. Maybe we mention in the contributions guideline where a PR should have been opened, but instead what the developer did was to do what he's used to do. I think I made the same mistake when I open my first PR to Horizon.

A possible solution could be to have a branch called "develop" (even make it the default in GitHub) and use it as the default development branch for Horizon which is the most active service anyways. Then for other services I'm okay having something like develop-horizonclient, develop-ticker and so on.

Another approach which I like more would be to move to trunk based development, where trunk is master and then use tags to cut releases. This means that master will contain unstable versions of each service, but I'd say that's fine, if you want a stable release use the tag or download from the releases tab.

[bartekn]

We had an internal discussion in the past about gitflow, copying my comment:

git-flow is great however I think it can be hard to separate breaking changes and new backwards-compatible features since everything is pushed to "develop" branch.

For example: there's a breaking change merged into "develop". When preparing the next release we will have to increment major number (x.0.0) of release name according to git-flow. We can also keep it in the feature branch until there are more breaking features we can merge together into "develop" but we can't see how they work together.

In the current branching strategy, it's actually possible to build multiple breaking changes together (and also merge "develop" from time to time) and check if these are compatible and release a new minor/major when we want to not when we have to. This would also support maintaining LTS versions if we ever decide to do that.

I think using master as a base branch for all the commits is fine (except for PRs containing breaking changes). We always check if master should be merged into a release branch before testing a release candidate so these commits would make it into a release. I also try to merge master into a release branch when I see a commit that should be included in the next release. #1953 was OK to merge into master.

A possible solution could be to have a branch called "develop" (even make it the default in GitHub) and use it as the default development branch for Horizon which is the most active service anyways.

It's a problem when breaking changes need to be merged. I'm afraid we'll need to do a lot of cherry-picking to create a stable release branch.

[leighmcculloch]

We can probably simplify our use of release branches for Horizon a little which would address your issue without burdening our process with a custom git flow.

My understanding is we've been using release branches to prepare changes for the next minor or patch release of Horizon, but that's probably unnecessary. We really only need to use a release branch if we're preparing a minor release for a previous major release, or a patch release for a previous minor release, or if there are breaking changes on master that we wouldn't want included in the next release of Horizon.

Example 1:
If we just released Horizon v0.23.0 and are planning to work on changes for v0.24.0, we should be okay with changes for Horizon v0.24.0 to go straight to master.

Example 2:
If we just released Horizon v0.24.0 and needed to patch v0.23.0, we should create branch release-horizon-v0.23.1 and make our changes to it instead of master.

---

Another approach which I like more would be to move to trunk based development, where trunk is master and then use tags to cut releases.

This is what we're doing today I think? Maybe Horizon is the only service not doing this. It's simple to have master be the development branch and have tags for production and I'm 💯for us continuing to do this for our non-Horizon code.

---

use it as the default development branch for Horizon which is the most active service anyways

I think we should avoid anything that causes our monorepo to center around one specific project even if ti is true that Horizon is the most actively developed code in here.

[bartekn]

@leighmcculloch I think what you're proposing is that master has all the changes including the breaking changes for the next minor (sigh, horizon versioning) or major. Our current strategy is that master includes only non-breaking changes and all breaking changes are done in release branch.

Your proposal makes sense except a few situations:

• Sometimes we do not want to include specific commits in master to be released in next minor. Ex. changes to shared packages (like support/*) or larger refactoring we want to test more before shipping in Horizon. If we revert them on the release branch and then merge it back to master (ex. we added a couple bug fixes during testing) the revert commit will also be included in master (afair). In other words: prefer cherry-picking over reverting.
• The original strategy for patch releases was to use master so that it included all of the latest non-breaking changes that were merged after last minor release (in the current strategy master should contain all non-breaking changes so that patch release includes a fix but also all of the latest non-breaking changes). In practice, we usually need to release patch quickly because of security/urgent issues. We branch off from the latest horizon release tag and include a single commit with the fix. We don't use master because there's no time to properly test all of the changes on master since the latest Horizon release. So we do it exactly like in Example 2.
• We sometimes work on two next releases in the same sprint. So we at least need a release branch for X.[curr + 2].0.

[abuiles]

@leighmcculloch I like your proposal and believe it would simplify the whole process, the reason why I mentioned a "develop" branch was to keep our current contract where master contains the stable versions of all the different services. But if we can get rid of that all together, even better.

@bartekn

Sometimes we do not want to include specific commits in master to be released in next minor. Ex. changes to shared packages (like support/*) or larger refactoring we want to test more before shipping in Horizon. If we revert them on the release branch and then merge it back to master (ex. we added a couple bug fixes during testing) the revert commit will also be included in master (afair). In other words: prefer cherry-picking over reverting.

What's the reason for not wanting to include those specific commits? I get what you are saying about the reverts, but maybe I just don't have strong feelings about reverts being present in the master timeline, at the end of the day our git log is a snapshot of our dev history and I think reverts are a normal part of the dev life cycle.

The original strategy for patch releases was to use master ...

Makes sense - I think Example 2 covers that scenario.

We sometimes work on two next releases in the same sprint. So we at least need a release branch for X.[curr + 2].0.

Could you elaborate a little bit more about this? How do we handle this scenario today? Also wouldn't this be the same as defining commit X as the final commit for release current + 1 ?

[leighmcculloch]

@bartekn Thanks for going into detail about why parts of the process work the way they do. I think this is really good the way it is. The use of the release branches is simple, easy to explain, and has some tangible benefits.

@abuiles I think we can change the base branch of contributor's PRs you did in #1953 and most contributors will have enabled the ability for us to push to their branch, so we can revert commits that are from master if we need to. We could try mentioning it in the contributing guidelines, but I think it will be overlooked, I think we just need to help people in the moment on the PR while we have this process.

When we move to multiple-modules we might need to change this process because changes in the SDK won't be able to be easily bundled with a release of Horizon. Once that happens there might be less advantage to using our existing process over a simple master.

[abuiles]

@leighmcculloch @bartekn I think we can close this and now this will issue will help us as "documentation" on why we use our current flow.