Automating/Streamlining the merging process

[martinhoyer]

With a goal to have finished PR's merged into main asap, while reducing manual workload for maintainers, let's brainstorm and discuss ways to accomplish them.

[martinhoyer]

Adopting autosquash-friendly commit messages with fixup! and squash

Inevitable typo and it's fix:

$ echo 'Helo' >> README.rst 
$ git commit README.rst -m "Adding Hello message to README"
[main d9e09a32] Adding Hello message to README
 1 file changed, 1 insertion(+)
$ sed -i 's/Helo/Hello/g' README.rst 
$ git diff
diff --git a/README.rst b/README.rst
index 43c1dad8..32954ffd 100644
--- a/README.rst
+++ b/README.rst
@@ -21,4 +21,4 @@ inheritance and elasticity metadata are organized in the structure
 efficiently, preventing unnecessary duplication.
 
 See the https://tmt.readthedocs.io web for detailed documentation.
-Helo
+Hello

Let's use git commit --fixup with a commit hash of the commit that we are fixing.
The message, while optional helps the reviewers (and you).

$ git log -1 README.rst
commit d9e09a32392e874d96e0124b3d8525408b5e7461 (HEAD -> main)
Author: Martin Hoyer <[email protected]>
Date:   Sat Apr 26 09:01:42 2025 +0200

    Adding Hello message to README
$ sed -i 's/Helo/Hello/g' README.rst ^C
$ git commit README.rst --fixup d9e09a32392e874d96e0124b3d8525408b5e7461 -m "damnit not a helicopter"
[main 241ee3e6] fixup! Adding Hello message to README
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git log -1 README.rst
commit 241ee3e6dac522253cd8c34aae7651ba0909237b (HEAD -> main)
Author: Martin Hoyer <[email protected]>
Date:   Sat Apr 26 09:04:15 2025 +0200

    fixup! Adding Hello message to README
    
    damnit not a helicopter

Note that the original commit message stays in place, while our new fixup message goes to second line - This allows describing our commits for duration of PR review process, while dropping the text during autosquash.

Let's rebase:

$ git rebase -i 65ce31f11789017f07e2756a91afbfeccd3a250e --autosquash
Successfully rebased and updated refs/heads/main.
$ git log
commit 8bbb8ce623cb27dab75e71e81d2b2ff829bb7e2b (HEAD -> main)
Author: Martin Hoyer <[email protected]>
Date:   Sat Apr 26 09:01:42 2025 +0200

    Adding Hello message to README

hint: git config --global rebase.autosquash true :)
hint: Git will interpret :/foo as “the most recent commit that contained the string foo in the first line of its commit message”.
git commit --fixup :/Hello would work instead of using the hash.

If you want to keep the message, it's exactly the same, just replace --fixup with --squash.
This might not be as suitable for automation, as it expects user interactivity, at least afaik.

$ sed -i 's/Hello/Chopper/g' README.rst
$ git commit README.rst --squash 8bbb8ce623cb27dab75e71e81d2b2ff829bb7e2b -m "Get to the choppaaa!"
[main 894236d1] squash! Adding Hello message to README
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git log -1
commit 894236d1955b66247bde35fbf840bba3afd5776b (HEAD -> main)
Author: Martin Hoyer <[email protected]>
Date:   Sat Apr 26 09:15:27 2025 +0200

    squash! Adding Hello message to README
    
    Get to the choppaaa!
$ git rebase -i 65ce31f11789017f07e2756a91afbfeccd3a250e --autosquash
[detached HEAD 3574eb39] Adding Hello message to README
 Date: Sat Apr 26 09:01:42 2025 +0200
 1 file changed, 1 insertion(+)
Successfully rebased and updated refs/heads/main.
$ git log -1
commit 3574eb39c370119398e72f40fdfb3440d55f7acb (HEAD -> main)
Author: Martin Hoyer <[email protected]>
Date:   Sat Apr 26 09:01:42 2025 +0200

    Adding Hello message to README
    
    Get to the choppaaa!

[martinhoyer]

I'm thinking that if we add it to Contribute doc and let frequent contributors aware, we can keep manual rebases to minimum and have clean commit history even if we use some for of automation (or at least streamilining) for merging process.

[martinhoyer]

Note that github auto merge with squash always creates only one commit, so it's not suitable for PRs where you want to keep 1+ commits, but need to squash some.

[martinhoyer]

Adopt GitHub Merge Queue for main Branch

What does Merge Queue do?

1. Queuing: Approved PRs are added to a queue instead of being merged immediately.
2. Validating: The queue sequentially tests each PR. Crucially, if main is updated while a PR is waiting or being tested (because another PR was just merged by the queue), the Merge Queue automatically re-evaluates the queued PR against the new latest version of main. It creates a temporary rebase using this newest main and runs the required checks on that prospective result.
3. Merging: Only if the required checks pass against the truly up-to-date main does the queue actually merge the PR. If checks fail (perhaps due to a conflict introduced by the newly merged PR), the problematic PR is removed from the queue for the author to fix.

Proposed Workflow

1. Prerequisites Met:

   • Pull Request has received ≥ 2 approvals.
   • The status | ready to merge label is added to the PR.

   Discussion Point: Who Adds the 'Ready to Merge' Label?
   Currently, it sounds like maintainers add the status | ready to merge label. We need to decide as a team:

   • Should this continue to be a maintainer-only action, signifying the final "go-ahead"?
   • Or, should PR authors also be allowed to add this label once they believe their PR meets all requirements (approvals obtained, manual prep done if using the 'Author Prep' alternative)?
   • If authors can add the label, should that directly allow the automated enqueueing Action to proceed (assuming approvals/CI are met)? Or is the maintainer adding the label serving as a crucial final, manual gatekeeper step before automation takes over?
   • This decision impacts the level of trust in the process and how strictly we want to control the exact point a PR is allowed to enter the automated merge pipeline.

2. Maintainer Prep (Manual):

   • (This is the default manual step in this workflow)
   • A maintainer checks out the PR branch locally.
   • If fixup! commits are present (or if squashing is otherwise desired), the maintainer runs git rebase -i --autosquash main (or equivalent command) to clean up the commit history and rebase the PR onto the latest main branch.
   • The maintainer resolves any conflicts that arise during the rebase.
   • The maintainer force-pushes the cleaned-up branch back to GitHub (using git push --force-with-lease is recommended).

3. Optional Alternative: Author Prep:

   • Concept: Instead of Step 2 being performed by a maintainer, the responsibility for cleaning up commit history can be shifted to the PR author before the status | ready to merge label is applied.
   • Adjusted Flow:
     1. Author prepares the final commits, using git rebase -i --autosquash main (or equivalent) to clean up history and ensure it's based on the latest main.
     2. Author force-pushes the final, clean branch.
     3. PR receives final approvals (reviewers check the clean history).
     4. status | ready to merge label is added (consider discussion point in Step 1).
   • Note: This approach requires clear contribution guidelines and authors comfortable with interactive rebase. It helps reduce the maintainer bottleneck.

4. CI Validation:

   • The force-push (performed in Step 2 by the maintainer, or earlier by the author in the alternative Step 3) triggers the required Continuous Integration (CI) checks on the PR's new head commit.

5. Automated Enqueueing (via GitHub Action):

   • Trigger: A GitHub Action is triggered when the required CI checks (associated with the workflow) complete successfully for the commit on the PR branch. Using workflow_run or check_suite completed events is recommended for the trigger.
   • Action Logic:
     • Verify the event context relates to a Pull Request branch.
     • Check via GitHub API: Does the PR have the status | ready to merge label?
     • Check via GitHub API: Does the PR have ≥ 2 approvals?
     • (Consider adding checks based on the label discussion in Step 1 - e.g., only proceed if conditions met AND label exists?)
     • If all conditions are met, the Action adds the PR to the GitHub Merge Queue using the GitHub API or gh CLI (e.g., gh pr merge --merge-queue).

6. Merge Queue:

   • The Merge Queue takes ownership of the PR.
   • It performs its final validation checks, ensuring the PR integrates cleanly with the absolute latest state of the main branch (including any PRs merged ahead of it in the queue).
   • If validation passes, the Merge Queue merges the PR into main.

Role of the status | ready to merge Label

The status | ready to merge label serves as a clear visual indicator that a PR has passed all manual reviews and automated checks and is ready to be added to the queue.

• Manual Trigger: A maintainer seeing this label knows they can confidently click "Merge when ready" to enqueue the PR.
• Potential Automation: We could create a GitHub Action that automatically adds a PR to the queue when this label is applied, but only if all other conditions (approvals, passing checks) are already met.

Is the label redundant if a maintainer adds it?

If the same person adds the label and clicks "Merge when ready", adding the label might seem like an extra step. However, the label is valuable as:

• A communication tool for the team, showing the PR's state at a glance.
• A prerequisite signal if we decide to automate adding PRs to the queue later.
• Won't be dropped if PR fails CI in merge queue

What about a status | in merge queue label?

GitHub already provides clear visual indicators in the PR interface when a PR is waiting in the merge queue or being actively tested by it. Adding a specific label for this state might be redundant. The key is having a clear signal (status | ready to merge) for when a PR is eligible to enter the queue.

Setup

Enabling the GitHub Merge Queue feature is just one checkbox and a few options in branch settings.

[martinhoyer]

Going further - Graphite Stacking, Merge Queue

This one is created by LLM, but it looks accurate to me (/me never used gt)
See:
https://graphite.dev/docs/merge-pull-requests
https://graphite.dev/docs/graphite-merge-queue
https://graphite.dev/docs/stacking-and-ci
https://graphite.dev/docs/cli-overview

Proposal: Adopt Graphite Workflow for Enhanced Development and Merging

This proposal outlines an alternative approach to our development and merging process, leveraging Graphite (graphite.dev) and its focus on stacked pull requests. This contrasts with the previous proposal centered around GitHub's native Merge Queue feature.

What is Graphite?

Graphite is a tool (CLI and web app) designed to work on top of Git and GitHub, primarily focused on improving the developer experience through stacked changes. Instead of creating large, monolithic PRs or many independent small PRs based off main, developers create stacks of dependent branches, where each branch represents a small, logical unit of work built upon the previous one.

• CLI (gt): Used locally to manage stacks (create branches, modify commits across the stack, rebase stacks, submit PRs).
• Stacking: Makes it easier to break down large features, get reviews on smaller pieces sooner, and manage dependencies automatically.
• GitHub Integration: Graphite syncs stacks to GitHub, creating individual Pull Requests for each part of the stack. These PRs are standard GitHub PRs.
• Merge Command (gt merge): A powerful command designed to merge an entire stack of PRs atomically and sequentially once all checks and approvals are met.

Proposed Workflow with Graphite

1. Development (Stacking):
   • Developers use the gt CLI to create stacks of branches (gt stack branch). Each branch builds upon the previous one in the stack.
   • Commits are made to the relevant branch within the stack. Changes can be easily modified across the stack using Graphite commands.
2. Submission:
   • Once a stack (or part of it) is ready for review, the developer runs gt submit.
   • Graphite creates/updates corresponding GitHub Pull Requests for each branch in the stack, automatically linking them and indicating dependencies.
3. Review & Approval:
   • Reviews happen on the standard GitHub PR interface. Reviewers can see the individual PRs and the context of the stack (often visualized better in the Graphite web UI linked from the PR).
   • Approvals (e.g., ≥ 2) are given on each individual GitHub PR within the stack.
4. CI Validation:
   • CI checks run on each individual PR in the stack, triggered by pushes/updates via gt submit. Graphite is aware of stack dependencies when interpreting CI results.
5. Merging the Stack:
   • Once the entire stack has the necessary approvals and all corresponding PRs have passing CI checks, a developer (or maintainer) runs gt merge.
   • Graphite verifies all conditions again.
   • It then merges the PRs sequentially, starting from the base of the stack, ensuring each merge happens against the correct, updated base. If any part fails (e.g., a last-minute check failure), the merge process for the entire stack is halted to maintain consistency.

Advantages of the Graphite Workflow

• Atomic Stack Merges: gt merge ensures that dependent PRs in a stack merge together sequentially and atomically, preventing broken states on main due to partially merged stacks. This serves a similar goal to GitHub's Merge Queue but is initiated client-side and specifically designed for stacked changes.
• Improved Code Review: Breaking large features into smaller, dependent PRs in a stack often leads to faster, more focused reviews.
• Smoother Development: Easier to manage dependencies and incorporate feedback across multiple related changes compared to juggling many independent branches.
• Reduced Rebase Conflicts (Potentially): While conflicts can still happen, Graphite's tooling can make managing rebases across stacks easier than manual Git commands.
• Clean History (Potentially): Stacking encourages smaller, logical commits, which can lead to a cleaner history on main after merging.

Potential Issues & Challenges

• Learning Curve: Developers need to learn the gt CLI and the concepts of stacking changes. This requires training and a shift in mindset from traditional feature branching.
• Tool Dependency: The workflow relies heavily on Graphite's tooling and service. While PRs are standard GitHub PRs, the full benefits (especially atomic stack merging) require using gt.
• CI Integration Complexity: Ensuring CI is configured correctly to understand and efficiently test stacked PRs might require adjustments (Graphite provides guidance, but it's an extra layer). See Stacking and CI.
• "Two Worlds" Problem: If only some developers adopt Graphite, managing the interface between stacked PRs and traditional PRs can be awkward. Merging stacks requires gt merge; merging individual PRs from a stack via the GitHub UI breaks the atomicity guarantee.

How Non-Graphite Contributors Interact

• External Contributors: They would submit standard GitHub PRs as usual. These would likely be reviewed and merged using standard GitHub mechanisms (potentially including the GitHub Merge Queue if implemented alongside). They would not be expected to use Graphite or submit stacks.
• Internal Contributors (Not using Graphite): They would also submit standard PRs. Maintainers might encourage them to adopt Graphite for larger changes, but their PRs would be handled outside the gt merge flow. They could review Graphite-submitted PRs on GitHub without issue. The main challenge arises if they need to collaborate directly on a branch that's part of someone else's stack.

Conclusion

Adopting a Graphite-centric workflow offers significant advantages for managing complex features and dependencies through stacking, potentially leading to faster reviews and more reliable merges via gt merge. However, it requires a commitment to learning a new tool and workflow paradigm. The key trade-off is between the power of stacking and atomic merges versus the universal familiarity and server-side simplicity of GitHub's native features like the Merge Queue. We need to assess if the benefits of stacking outweigh the learning curve and potential friction with non-Graphite users/contributors.

[martinhoyer]

CI

Lighter, reliable, granular, parallel?

Currently there is many rpms built, many tests run on every single commit. What do you think about making things a bit more granular other than test and full test ?

• Level 1 can be just "fedora:latest" and "centos:latest" rpms with subset of tests that finish fast and are reliable
• Can we add a parallel execution of another sub-set of tests against the same rpms to have higher test coverage with same execution speed?
• Level 2 could add more tests, but only if Level 1 passes
• Level 3 would run everything, de facto current full test label
• Automatically (or at least manually) run subset of tests based on context of changed code? i.e. always run all provision tests when touching /tmt/steps/provision/ files.

Run faster?

I believe it would be beneficial to spend some time identifying and resolving bottlenecks and low-hanging-fruit that make the pipelines run for so long. Without sacrificing confidence of course.
Cache here, duplicate there...

---

I guess it boils down to totally overhauling the tmt tests by getting rid of beakerlib and shell as much as possible and making it approachable to all potential contributors, be it with supporting Pytest as tmt test framework and using that or other solution.
To me at least, this is currently a biggest pain point in contributing to tmt.

[thrix]

Also note that Testing Farm has this implemented by their own using Python in tft-admin tool and enabled for their downstream repositories on gitlab.cee. It saves us a lot of clicking, you can see how busy the bot account is each working day:

https://gitlab.cee.redhat.com/ci-bot

The bot is implementation is fairly simple:

1. the bot goes through all open MRs in a forever loop (run in openshift cronjob for each project)
2. based on the Testing Farm policy (certain labels set on the MR) it decided if the MR can be merged or needs to rebased
3. it limits the number of rebases based on the gitlab ci pipeline state to mitigate overloading the CI with too many rebases

The cronjobs exist for each repository and monitored branch:

• baseos-qe/citool-config (staging branch) - cronjob
  https://gitlab.cee.redhat.com/baseos-qe/ansible-baseos-ci/-/blob/main/openshift/tasks/merge-workflow-ansible-baseos-ci.yaml?ref_type=heads
• similar config for citool-config (production branch) and ansible-baseos-ci repository

The implementation is in tft-admin, gitlab command, you can see the workflow here:

https://gitlab.cee.redhat.com/baseos-qe/ansible-baseos-ci/-/blob/main/cli/tft/commands/gitlab.py?ref_type=heads#L80

We had plans to do the same for tmt's GitHub project with @lukaszachy, as the above solution can be tailored the similarly for GitHub projects. Btw we are very happy with it after 2+ years in production. Here is the tracking TFT issue: https://issues.redhat.com/browse/TFT-2520. Unfortunately it never got enough priority to do. If we decided this is the way to go, we can try to put that issue in the TFT sprint.