Stacks-Core Branching Structure Proposal

[jferrant]

One of the biggest challenges with working with the stacks-core codebase is the current branching structure/merge strategy.
This discussion is to propose some improvements to the current workflow and to discuss the pros and cons.

Proposed branch strategy:
MAIN -> this is the release branch
DEVELOP -> This should be all changes slated for release (this essentially is a testing branch) (preferably this should be named STAGING)
NEXT -> Built on DEVELOP plus hardfork changes (preferably this should be called DEVELOP)

One issue is that NEXT and DEVELOP often diverge substantially resulting in out of date code with a plethora of merge conflicts. To prevent these conflicts growing to unmaintainable amounts, if someone merges into DEVELOP, there should be an immediate (maybe automatically triggered?) PR opened into the NEXT branch. This will ensure that NEXT is always up to date with DEVELOP.

Additionally, due to the fact that both NEXT and DEVELOP contain multiple merge commits, there is no linear history. This makes it impossible to bisect for bugs/have proper code attribution/makes it impossible to properly rebase feature or chore branches.
If we enforced rebasing (preferably with squash commits on all PRs), we would have linear history with all of the advantages listed above. Unfortunately github does not provide a rebase button so these must be done manually in your own local terminal/IDE. However, github does provide a squash merge button and can enforce linear history. I highly recommend enabling this and making it default.
-> In protected branches, enable "require linear history" and it will enforce rebasing and squash commits https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#require-linear-history
-> For enabling squash/rebase: see https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github for how to configure the PR configurations

Please comment below if there are other improvements you think should be mentioned or if you have issues with this proposal.
I am not 100% sure what the actual intended purpose of DEVELOP vs NEXT is so clarity on that would be great.

@wileyj @xoloki @soju-drinker @obycode @jcnelson @kantai @AshtonStephens

[kantai]

Proposed branch strategy:
MAIN -> this is the release branch
DEVELOP -> This should be all changes slated for release (this essentially is a testing branch) (preferably this should be named STAGING)
NEXT -> Built on DEVELOP plus hardfork changes (preferably this should be called DEVELOP)

Is this very different from the scheme in place but with some renames? If so, I think staging is a confusing name. The current naming scheme is essentially an extension of gitflow (https://datasift.github.io/gitflow/IntroducingGitFlow.html) -- the develop branch is the same semantically as the develop branch in gitflow. The problem with replacing develop with a staging branch is that hotfixes do not require releasing through develop: a hotfix merges directly to master (or main), so that develop isn't really a "staging branch", but rather a branch for the next feature release. The reason a gitflow extension is required in this repo is that managing a hard fork really does require separating the hardfork branch from the next feature release.

One issue is that NEXT and DEVELOP often diverge substantially resulting in out of date code with a plethora of merge conflicts. To prevent these conflicts growing to unmaintainable amounts, if someone merges into DEVELOP, there should be an immediate (maybe automatically triggered?) PR opened into the NEXT branch. This will ensure that NEXT is always up to date with DEVELOP.

I completely agree with this, and think that an automated trigger to open a PR and assign it to the prior PR's owner is necessary to enforce this.

Additionally, due to the fact that both NEXT and DEVELOP contain multiple merge commits, there is no linear history. This makes it impossible to bisect for bugs/have proper code attribution/makes it impossible to properly rebase feature or chore branches.
If we enforced rebasing (preferably with squash commits on all PRs), we would have linear history with all of the advantages listed above. Unfortunately github does not provide a rebase button so these must be done manually in your own local terminal/IDE. However, github does provide a squash merge button and can enforce linear history.

I don't have a particularly strong opinion on this-- I'd be fine either way with the repo enforcing this.

[wileyj]

Is this very different from the scheme in place but with some renames? If so, I think staging is a confusing name. The current naming scheme is essentially an extension of gitflow (https://datasift.github.io/gitflow/IntroducingGitFlow.html)

Isn't there a missing intermediary branch between develop and master in the gitflow model?
today, we have feat/branch -> develop -> master, but perhaps it should be something like feat/branch -> develop -> staging -> master? I suppose you can argue that a tag is the release branch (staging in the previous sentence), but today the tag is only created when a release builds.

It also appears that if we started to enforce linear commits/squash merges - it could allow us to automate that PR of develop -> next rather than only opening a PR for manual merge.

[jcnelson]

I completely agree with this, and think that an automated trigger to open a PR and assign it to the prior PR's owner is necessary to enforce this.

I'm generally onboard with this as well. It won't remove all merge conflicts, but it'll cut down on their frequency.

I don't have a particularly strong opinion on this-- I'd be fine either way with the repo enforcing this

I'd like to know more about the advantages/disadvantages of this. Does it reduce the number of merge conflicts?

[ldiego08]

@jferrant However, github does provide a squash merge button and can enforce linear history. I highly recommend enabling this and making it default.

This is a fantastic suggestion. Squash Merges leaves PRs as a single, self-contained unit of work that can be more cohesively referenced in the future while keeping the history clean. Sometimes, developers do small commits with fixes like "chore: fix comment typo" and these end up polluting the parent branch if committed linearly. Those commits are better off encapsulated in the context of the PR while having one big squash commit with a summary (which can be auto-generated by GitHub) on main or develop.

Just be aware that squash merging, while it's more convenient history-wise, can require risky rebases and force pushes of the PR or the parent branch if GitHub is not able to do it automatically.

@kantai If so, I think staging is a confusing name.

"staging" is often more appropriate for apps deployed to a staging environment.

@jcnelson I'd like to know more about the advantages/disadvantages of this.

IMHO, the more you automate rules the less burden it is to maintain order and the more streamlined the developer experience is.

Among other thoughts, I'm not sure if Stacks is using feature flags or anything similar? They are quite useful to keep risky features from breaking the existing code even if accidentally committed to main and deployed - also super easy to implement.

[cylewitruk]

There's a fair bit of information in CONTRIBUTING.md in regards to the current branching strategy, but to summarize:

• master/main is the current "release". hotfixes should be made here and immediately propogate up to develop and next, but regular development should not occur here.
• develop is the next non-consensus-breaking feature release - this is probably where the majority of contributors should fork from.
• next is the next consensus-breaking release.

It took me a little bit to really "get it" how things were setup, but after getting used to it I think the setup in principal is actually pretty good. I like that there's a separate development cycle for non-consensus-critical and consensus-critical paths. The problem is that these branches aren't synced often enough and we end up with giant & risky merges.

I think this structure could work quite well GIVEN:

• we constrain master to ONLY releases and hotfixes,
• any updates to master are immediately propogated up to develop and next,
• any updates to develop are immediately propogated up to next,
• the versioning scheme is clear (for example, major = consensus breaking a.k.a. next, minor = feature a.k.a. develop, patch = fix a.k.a. increment on master patch version)

On squashing I'm generally a proponent of squashing PR's because I personally use git as a bit of a undo-tool when I'm developing new features where I'm not sure how exactly it'll pan out code-wise. I'm probably not alone there ;) PR's should aim to be small enough that the feature can be described in a line or two.

Re: commit messages (this is just a thought), there's a lot which can be done to help ensure uniformity - one idea would be enforcing Conventional Commits via i.e. commitizen, which can use pre-commit-msg hooks in git to ensure that a commit meets the guidelines before the commit is actually accepted by git. Using commitizen you can also generate incremental changelogs etc. as part of a workflow.

[jbencin]

In protected branches, enable "require linear history" and it will enforce rebasing and squash commits

It's easy to keep a linear history if you only have one persistent branch. If you only have master, you always branch from there and squash merge back. But can we maintain a linear history with 3 persistent branches (master, develop, next)? How would you propagate a change to all three branches without merge commits?

• Separately squash merge each PR into each branch?
• Constantly rebase develop and next off master to update them?

[xoloki]

Every commit should be a PR, which squashes onto a freshly rebased branch, whatever branch that may be.

That squash should be cherry picked wherever it needs to go, thereby maintaining linear history. Remember, a rebase is just cherry picking commits onto a base branch.

Does that make sense?

[xoloki]

So basically I'm saying to do your first option. What I envisage is that PRs go into develop if they aren't consensus breaking, and into next if they are. Once a PR goes into develop it gets cherry picked to next.

When we want to release a non-consensus breaking version we cherry pick everything from develop onto main. Ditto for next and consensus breaking changes. So squashed commits flow into main from develop or next.

[saralab]

From @cylewitruk :
Having everyone use cargo fmt --all -- --config group_imports=StdExternalCrate,imports_granularity=Module as well (if that's the config we want) would help with using statements

[cylewitruk]

Or we can create a .rustfmt.toml file at the project root so no special commands are needed: https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=

[cylewitruk]

I just tried this:

unstable_features = true
reorder_imports = true
imports_granularity = "Module"
group_imports = "StdExternalCrate"
reorder_modules = true

Which uses unstable features, so nightly has to be used :/ This limitation doesn't exist when passing the parameters on the command line.
cargo +nightly fmt

[cylewitruk]

Just to follow-up on this, a cargo fmt-stacks alias has been added to the stacks-blockchain repo which handles this functionality :) So use fmt-stacks instead of fmt moving forward.

[obycode]

I'm a fan of rebases over merge commits as well. I find requiring squash commits can be a bit overkill, as there are often legitimate reasons for multiple separate commits as part of one PR.

I strongly support the automation of master -> develop and develop -> next PRs.

[cylewitruk]

I agree that requiring squash can be overkill. But I think that we should have some sort of guideline/policy (as well as a how-to example) to squash commits down to the minimum commits needed for a PR. Too many commit messages that don't add much value also reduce the readability even in a linear history. I personally commit very often and often with scrap messages, just so I have something to rollback to if my next experiment doesn't work out, so I definitely squash - but this could be multiple squashes encompassing different functionalities just to keep the commit log clean prior to PR.