[RFE] Standardize Pull Request, Merging workflow and branch protection rules

[John15321]

In the Flatcar organization, many repositories still lack branch-protection rules, and Pull-Request workflows vary widely from repo to repo. Below is a unified configuration proposal that can be applied to most repositories (some may require special settings).

---

1. Default branch

• Set the default branch to main (or master).
  Most modern tools default to main, so choosing main simplifies integrations.

---

2. Pull-Request settings

• Allow squash merging only
  Disallow “Merge commits” and “Rebase merging.” Squash merging produces a single commit per PR on the default branch, yielding a clean, linear history. The commit message defaults to the PR title + description, and the squash commit links back to the PR for traceability.
• “Suggest updating branch”
  Encourages contributors to rebase their branch onto the latest default-branch changes. (You can later require branches to be up to date.)
• Enable auto-merge once all requirements are met (approvals, CI).
• Automatically delete head branches after merge to keep the branch list tidy.

---

3. Branch ruleset for the default branch

Create a ruleset targeting your default branch.

---

4. Bypass permissions

Maintainer-level teams should be granted “bypass” privileges so they can override rules when necessary.

---

5. Target branches

Set the rule’s target to your default branch (e.g. main).

---

6. Individual rules

1. Restrict deletions — prevent accidental branch deletion (default ON).

2. Require linear history — forces contributors to rebase before merging.

   

3. Require review from Code Owners

   • Ensures designated owners sign off on changes.
   • Discussion: flatcar/Flatcar #1665
   • Learn about CODEOWNERS: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

4. Require approval of the most recent reviewable push — guarantees the final approved commit is what gets merged.

5. Require conversation resolution before merging — ensures all review comments are addressed.

6. Allowed merge methods: Squash

   

7. Require status checks to pass — CI must succeed before merge.

8. Require branches to be up to date before merging — enforces rebasing onto the latest default branch.

   

9. Block force pushes — (optional; default ON; open for discussion).

   

10. Require code-scanning results — integrate your chosen code-scanning tool on every repo.

    

Note: These settings serve as guidelines—or “rails”—to keep our workflow consistent and maintain a clean history. Maintainers retain the ability to bypass or adjust any rule when exceptional circumstances arise.

[John15321]

List of repos without main branch protection:

• https://github.com/flatcar/ue-rs
• https://github.com/flatcar/flatcar-cloud-image-uploader
• https://github.com/flatcar/flatcar-terraform
• https://github.com/flatcar/container-linux-config-transpiler
• https://github.com/flatcar/locksmith
• https://github.com/flatcar/sysext-bakery
• https://github.com/flatcar/flatcar-demos
• https://github.com/flatcar/update-ssh-keys
• https://github.com/flatcar/flatcar-dev-util
• https://github.com/flatcar/flatcar-build-scripts
• https://github.com/flatcar/mayday
• https://github.com/flatcar/flatcar-release-mirror
• https://github.com/flatcar/flatcar-packer-qemu
• https://github.com/flatcar/updateservicectl
• https://github.com/flatcar/flatcar-cpe-util
• https://github.com/flatcar/flatcar-maintainer-private

[tormath1]

Thanks, this all looks good to me. Question: can we set this from the template repo? To avoid thinking about this when creating new repos?

I just have some concerns regarding the "squash & merge":

• it does not preserve the signature of the commit author but it uses the GitHub signature (e.g here: tormath1/scripts@35297bb)
• for some features, it's easier to split development into commits to use later. E.g when upgrading a package on ::coreos-overlay, we usually have one commit: sync with Gentoo and second apply Flatcar downstream modifications the last one would be lost when squashing so for the next upgrade it could be challenging to retrieve those downstream modifications.
• having split commits can ease the revert of some specific part of a feature implementation (search for git log --grep Revert in Mantle or Scripts project for example)

For the "Block force pushes — (optional; default ON; open for discussion)." - it does apply only on main branch right?

[John15321]

Thanks, this all looks good to me. Question: can we set this from the template repo? To avoid thinking about this when creating new repos?

I just have some concerns regarding the "squash & merge":

• it does not preserve the signature of the commit author but it uses the GitHub signature (e.g here: tormath1/scripts@35297bb)
• for some features, it's easier to split development into commits to use later. E.g when upgrading a package on ::coreos-overlay, we usually have one commit: sync with Gentoo and second apply Flatcar downstream modifications the last one would be lost when squashing so for the next upgrade it could be challenging to retrieve those downstream modifications.
• having split commits can ease the revert of some specific part of a feature implementation (search for git log --grep Revert in Mantle or Scripts project for example)

For the "Block force pushes — (optional; default ON; open for discussion)." - it does apply only on main branch right?

Hey @tormath1! Thanks for taking a look at this 😄

When it comes to using the GitHub Templates to spread the configuration - as far as I know unfortunately no. I know people sometimes do it via fancy GitHub actions but thats about it

As for signatures on commits - my honest response is that I dont know how to fix this issue. I will try to research more on this topic and get back to you.

When it comes to splitting development. Personally I have always been of the idea that a commit to main/PR, should represent one, singular thing, thus being easy to revert. But this is of course very abstract and sometimes it will not make sense for a given project, so I would say that in that case the solutions for that would be to revert the change, and then use the branch to create a new one, extract wanted commits and create a new PR/merge into main. - But yeah its not ideal

For the "Block force pushes — (optional; default ON; open for discussion)." - it does apply only on main branch right?

I think if the main is the default one, then it applies to main?

[sayanchowdhury]

Pt. 2, IMO, I’m happy with the current system as it helps preserve history — provided the PR commits are properly arranged. Additionally, it makes reverts easier. We should establish guidelines on how to organize PRs and commits. I’d also recommend avoiding auto-merging PRs, even if all checks pass. Merging should be done manually to provide a minimal gating layer.

Pt. 6, I believe we should ensure that PRs do not include merge commits. I noticed some PRs had them, and in such cases, we should ask contributors to tidy up the commit history. And Pt. 6.5:
It can remain disabled — I'm a bit divided on that.

Everything else looks good.

[John15321]

Pt. 2, IMO, I’m happy with the current system as it helps preserve history — provided the PR commits are properly arranged. Additionally, it makes reverts easier. We should establish guidelines on how to organize PRs and commits. I’d also recommend avoiding auto-merging PRs, even if all checks pass. Merging should be done manually to provide a minimal gating layer.

Pt. 6, I believe we should ensure that PRs do not include merge commits. I noticed some PRs had them, and in such cases, we should ask contributors to tidy up the commit history. And Pt. 6.5: It can remain disabled — I'm a bit divided on that.

Everything else looks good.

I understand the argument for avoiding auto merges. Personally im 50/50 split on this - so I dont mind not using that.

Apart from this are there any other things you would change in order to implement it in all of Flatcar? @sayanchowdhury

[chewi]

I don't usually have strong opinions on these things, but I really don't want to force squashing. I always strive to have clean separate commits with good descriptions for each. This helps me enormously, as I look back through the history very frequently. I encourage others to do the same, but I recognise that not everyone works the same way, and some find git difficult to handle. I therefore think that squashing should still be an option for times when pushing for a cleaner history would lead to a negative experience for the contributor.

I don't really mind about merge commits. I understand them, but they can be confusing for others. Gentoo's main package repository bans them because they don't make sense when the changes made by different people are largely unrelated, i.e. does it really matter what the state of app-editors/nano was when sys-apps/systemd was modified? Most changes to flatcar/scripts relate to packages, so we could make the same argument there.

I force push to my own branches all the time. I have force pushed to main on very rare occasions when I thought I could get away with it, but I don't think that's sensible for Flatcar. If we really need to do it, we could temporarily change that rule following a discussion.

[ader1990]

Squashing, for example in the case of flatcar/scripts#2300, would have broken some pretty good future understanding / scenarios like: revert one commit if needed, track the commit that was good or wrong, see the big picture in a better way as it is split in rather good atomic parts.

[ader1990]

Pt. 2, IMO, I’m happy with the current system as it helps preserve history — provided the PR commits are properly arranged. Additionally, it makes reverts easier. We should establish guidelines on how to organize PRs and commits. I’d also recommend avoiding auto-merging PRs, even if all checks pass. Merging should be done manually to provide a minimal gating layer.

Pt. 6, I believe we should ensure that PRs do not include merge commits. I noticed some PRs had them, and in such cases, we should ask contributors to tidy up the commit history. And Pt. 6.5: It can remain disabled — I'm a bit divided on that.

Everything else looks good.

I would suggest that the Flatcar merging workflow uses the dual responsibility of merge: currently, the maintainer owner of the PR needs to merge it. I would change this to have another maintainer merge a maintainer's PR, to share responsibility. This dual sharing of merge responsibility is used in almost all of the OpenStack projects.

[jepio]

Definitely no to squash merges. Individual commits group an isolated diff with a matching description. This is super valuable when reviewing history for a long lived project like Flatcar. Additionally the historical context/sequence of changes within a branch helps understand the interactions between commits. Squash merging loses all of that.

Enforcing merge commits keeps all that information. It is also possible to revert a whole merge commit or each individual commit if the rest is fine.

So I'd say we should allow only merging through merge commits. git log --first-parent --oneline provides a linear condensed view when needed and you still have the option to drop into individual commits.

And we shouldn't be asking people to rebase branches by default either - only when necessary: when there are conflicts or dependencies or to cleanup commits. Rebasing invalidates any testing that a contributor did prior to the rebase, and the point at which a branch was started is also useful information.

I like that people are looking at the contributor experience, so we should tolerate merge commits in external PR branches or maintainers should clean up history when needed, but not put that burden on contributors. Internally we should keep the history within each branch at a high standard: reasonable split into (small-ish) commits, good commits messages, no WIP/TMP/attempts. This investment pays dividends later down the line for us.

[John15321]

Pt. 2, IMO, I’m happy with the current system as it helps preserve history — provided the PR commits are properly arranged. Additionally, it makes reverts easier. We should establish guidelines on how to organize PRs and commits. I’d also recommend avoiding auto-merging PRs, even if all checks pass. Merging should be done manually to provide a minimal gating layer.
Pt. 6, I believe we should ensure that PRs do not include merge commits. I noticed some PRs had them, and in such cases, we should ask contributors to tidy up the commit history. And Pt. 6.5: It can remain disabled — I'm a bit divided on that.
Everything else looks good.

I would suggest that the Flatcar merging workflow uses the dual responsibility of merge: currently, the maintainer owner of the PR needs to merge it. I would change this to have another maintainer merge a maintainer's PR, to share responsibility. This dual sharing of merge responsibility is used in almost all of the OpenStack projects.

Thanks for laying this out so clearly! I agree with your points 😄

And I really like your suggestion about dual responsibility for merging maintainer-authored PRs. That kind of peer review and shared accountability aligns well with how other mature projects operate and could be a great fit for us too.