New RFC: Use Trunk-based Development

[edmundmiller]

Have you read the RFC docs?

• Yes, I have read and understood the RFC docs

Summary

Instead of GitFlow, we should adopt Trunk-Based Development.

A source-control branching model, where developers collaborate on code in a single branch called ‘trunk’, resist any pressure to create other long-lived development branches by employing documented techniques. They therefore avoid merge hell, do not break the build, and live happily ever after.

Champion

@edmundmiller

Background & Motivation

An example where GitFlow is confusing and doesn't help the developer. nf-core/bamtofastq#126

It ends up creating a lot of extra noise, and work for the developer and review fatigue of PRs that aren't actually making changes and are just a git ceremony.

Issues:

• Release bottlenecks: Multiple approval layers between dev and master slow feature delivery
• Contributor confusion: dev functions as the de facto main branch while master remains the official default
• Maintenance burden: Managing three branches across 100+ pipelines creates substantial overhead

Scientific Software Alignment

• Lower barriers for academic contributors who may have limited version control experience
• Faster validation of scientific methods through continuous integration
• Improved reproducibility tracking with a single source of truth
• Better alignment with container-based workflows increasingly adopted in bioinformatics

This will allow:

• Pipeline releases are created more frequently and aren't held up by "dev => master" review requests, template updates, and module version updates.
• We can bump the dev reviews up to 2 people to increase the actual stringency of the reviews and not rely on a few people to review thousands of lines before a release can be cut.
• Avoid merging master => dev and other back-and-forth git syncs that are confusing.

Goals

• nextflow run nf-core/sarek -profile test works without -r master or a revision locally for users.
• Remove the dev and master branches and migrate to main branches

Non-Goals

• Implement the new review rules to merge to main.
• Decide who can cut a release.

References

• Example repo
• Trunk Based Development
• Nextflow Issue

[nf-core-bot]

RFC approval status: 🕐 Pending

RFC has approvals from 1/7 required @core-team quorum.

Review Status	Core Team members
✅ Approved	@maxulysse
❌ Rejected	@mashehu
🕐 Pending	@ewels, @mribeirodantas, @JoseEspinosa, @mirpedrol, @FriederikeHanssen, @kenibrewer, @jfy133, @edmundmiller, @sateeshperi, @FranBonath, @christopher-hakkaart, @nvnieuwk

[maxulysse]

/approve

[mashehu]

/reject

too micro-managing imo. people here are usually not so familiar with different dev approaches and should develop however fits best for them and that is usually git flow.

having the three fixed branches actually makes creating rulesets and checking them quite easy.

A simpler solution to most of these points would be to encourage small releases (like we do for tools 😜 🙃)

[bentsherman]

I would propose something more lightweight, which is to simply encourage as best practice to merge PRs via squash or rebase, i.e. enforce linear history. That would make it easier to review the commit history, triage when something breaks, etc.

But I agree in principle with @mashehu that at the end of the day devs should be free to dev however they want. I would only push for the flagship pipelines to follow whatever we think the best practice is

[edmundmiller]

Right now we require the three branches https://nf-co.re/docs/guidelines/pipelines/requirements/git_branches

[maxulysse]

Agreeing with Ben, we need lightweight solution that will still allow for template update while not being forced on developers.

[famosab]

One could also think about using https://www.conventionalcommits.org/en/v1.0.0-beta.2/ for the PR titles and then having release-please automatically create the Changelog while always merging to main instead of dev. I found that this reduces overhang quite a lot and as long as the PR title is properly checked and the merge is done via squash and merge you get a clean changelog. And people can still find the current stable released version via the tag.

[FriederikeHanssen]

I gues it moves the review burden to the left. At the moment (especially for first releases) people can develop pretty fast without being stuck by waiting for reviews much, but it comes at the price of having a huge first release PR that can be stuck for a while, to lesser extend that is also tru for subsequent releases.

I am therefore not sure, I totally get these two points:

Lower barriers for academic contributors who may have limited version control experience
Faster validation of scientific methods through continuous integration
We can bump the dev reviews up to 2 people to increase the actual stringency of the reviews and not rely on a few people to review thousands of lines before a release can be cut.

I might be wrong and enough people are able to regularly review. But I also don't want people while they actually have time to develop, maybe coming out of a hackathon , to be stuck in #request-review channel waiting for two reviews for every PR to get a base pipeline going.

[muffato]

I don't have voting rights, but I'm really interested in this topic, and I have a very different take on @edmundmiller 's description of the issues and the proposed changes.

Issues:

• Release bottlenecks: Multiple approval layers between dev and master slow feature delivery

In our pipelines, the two approval/review layers focus on different things. Contributions to dev tend focus on the code / feature itself. PRs to master are about getting everything in order before the release, like updating the contributor list, double-checking the docs, the extra CI checks, etc. A PR to master often summarises work done across multiple PRs to dev, and is therefore more efficient than having every single source PR be fully compliant in terms of documentation, metadata, etc. I think the RFC would have the opposite effect and increase the workload of the contributors.

There is also an easy way of avoiding the two layers. The template already allows PRs to main from a patch branch. That was intended for bugfixes, but it can be abused to implement your model (with the constraint of only 1 patch PR being possible at a time).

• Contributor confusion: dev functions as the de facto main branch while master remains the official default

If dev is complete and working, what's holding doing a release, then ? There shouldn't be any merge conflicts between dev and main.
PRs to main are about checking and maintaining the quality of the entire repository and all the pipeline assets. If these are already properly checked when doing PRs onto dev, then the PRs onto main are very straightforward and quick.

• Maintenance burden: Managing three branches across 100+ pipelines creates substantial overhead

Scientific Software Alignment

• Lower barriers for academic contributors who may have limited version control experience

True, and it doesn't help that GitHub uses the default branch for all PRs. But all it needs is the maintainer to go and change the target branch if someone got it wrong.

• Faster validation of scientific methods through continuous integration

All the above said, I agree that if you follow all the rules when doing a PR to dev, the PR to master, does feel unnecessary and makes things potentially slower. But the key question is: were all the rules followed when doing a PR to dev ? There's an argument of speed vs quality.

• Improved reproducibility tracking with a single source of truth

If someone is ok using the dev, they'll be equally ok using a feature branch. I would even argue that a branch named better_mapping inspires more confidence than a branch named dev.
Even with trunk-based development, there still isn't a single source of truth.

• Better alignment with container-based workflows increasingly adopted in bioinformatics

I'm not sure what this means ?

This will allow:

• Pipeline releases are created more frequently and aren't held up by "dev => master" review requests, template updates, and module version updates.

Holding up a release because of template / module version updates is a decision from the maintainer, not a requirement of the git model.
Like many maintainers I'm attached to giving a "sense", "goal", or "purpose" to a release

• We can bump the dev reviews up to 2 people to increase the actual stringency of the reviews and not rely on a few people to review thousands of lines before a release can be cut.
• Avoid merging master => dev and other back-and-forth git syncs that are confusing.

git merges are in theory inevitable if you've got more than 1 feature branch because there's no guarantee that the two features won't conflict with each other. Regardless of trunk-based development or not.
And i you only ever have 1 feature branch, then you can already use the patch model and call it a day.

In the current model, dev is almost always be in front of master. The only time it could be behind is if you push from patch to master to cut a bugfix release while some features are pending in dev. But in that situation, the question is: why was the patch not pushed to dev and then dev released ?

---

To be fair, I think my point of view is largely the result of working for many years with people whose job is to develop pipelines, adhere to the guidelines, etc. They learned the git model and are now following it. Our pipelines don't get that many contributions from 3rd-parties who are not familiar with the development model.

Don't take me wrong, I've also been in positions where we used the dev branch or a feature branch in production for far too long. But that wasn't caused by the git model, the same would have happened with trunk-based development. Really it was bad project / software management from our side. To @mashehu's point, releases don't have to be big or super meaningful. Small releases are OK.

Overall, I don't see this as a "git flow" discussion but as discussion on the release model, i.e. doing rolling releases or not. git is there to support the release model, not the opposite.
In my experience, rolling releases demand a lot more scrutiny and quality checks.

[nf-core-bot]

Approval status: 🕐 Pending

RFC has approvals from 1/7 required @nf-core/core quorum.

Review Status	Core Team members
✅ Approved	@maxulysse
❌ Rejected	@mashehu, @jfy133
🕐 Pending	@ewels, @mribeirodantas, @JoseEspinosa, @mirpedrol, @FriederikeHanssen, @kenibrewer, @edmundmiller, @sateeshperi, @FranBonath, @christopher-hakkaart, @nvnieuwk

[apeltzer]

Also chiming in here a bit with some different perspective. We have cases where we have people or a team working on pipelines for us, contributing directly back to existing pipelines, which became our default way of doing things with nf-core.

Maintainers are not always 100% happy (or in disagreement) with proposed changes and thus there is a lot of back and forth - making it difficult to move relatively fast and battle test developments in a separate branch (e.g. feature branch) before going into broader discussions with the main developer of said pipeline. As we're working in regulated environments, stability and reliability of a release is a requirement for us (insert sad cowboy here) - but having multiple branches with features / requirements laying around for months is not directly helpful either (as we cannot start testing / validating based on multiple branches) - and we really end up in the situation that is described in Edmunds initial post in this thread (going back to merge changes from master to said feature branch, etc).

The problem really appears in the details of the daily working model here:

If we keep changes (to dev or later to main) atomic / or small to review, we end up DDoSing said main developer as the main dev gets hit with potenziall dozens of PRs from a team of devs for weeks.

If we don't have small PRs, we end up having said dev looking through potentially many commits / changes ("the classic huge release PR") at once whenever we are aiming for a release to get things into a stable release branch. Neither nor is ideal and I'm at a loss whats preferable over the other or easier per se?

The working model with devs you hire however is that you pay per hour within an agreed time period (60 hours over 3 months now). If we are waiting > a few days for PR reviews, its getting tricky to get much done. Of course can also have some second dev going through code and doing the PR within their 60 hours, but the main dev of said pipeline might not accept this either, so it boils down to going through the main developer either way. No blaming at all, its just the user experience right now.