Skip to content

Conversation

@yadij
Copy link
Contributor

@yadij yadij commented May 18, 2025

Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Without this configuration update github merge queues will
auto-reject all PRs attempt to merge.

Github merge queues are now able to perform all the required
(slow.yaml) checks and details Squid Project applies to code
additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project master branch.

v7 branch is listed for Squid Software Maintainers use on the
current release branch.

v6 and older branches are deprecated and merge rejection
is a desirable outcome there.

For now Anubis and 'auto' branch are left enabled.
They can be dropped later.
@yadij
Copy link
Contributor Author

yadij commented May 18, 2025

When PR #2067 and this PR are both merged we should no longer need Anubis.

@kinkie
Copy link
Contributor

kinkie commented May 18, 2025

nice!

@yadij yadij force-pushed the ci-merge-queue branch from 94f92db to e2586be Compare May 19, 2025 10:34
@rousskov rousskov self-requested a review May 19, 2025 14:07
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description: Github merge queues are now able to perform all the required checks and details Squid Project applies to code additions.

When PR #2067 and this PR are both merged we should no longer need Anubis.

Do GitHub merge queues allow immediate merge of a unanimously approved PR while waiting 10 days for a PR approved by one core developer and 48 hours for a PR approved by two core developers? Do GitHub merge queues use PR title/description as the commit message and validate its formatting like Anubis does? These two questions do not enumerate all useful Anubis features. A comprehensive comparison of features should be performed (and these questions can be used as a starting point).

Overall, Anubis does a lot more than creating a staged commit (for "slow tests"). Also, GitHub probably uses different metadata (commit message and authors) for the staged commit. If we replace Anubis with GitHub merge queue, we will lose several features and protections that we frequently use. I am happy to collaborate on removing Anubis if others think Anubis is a problem, but that removal should start with a problem statement and a careful evaluation of the resulting changes to our PR handling process.

@yadij
Copy link
Contributor Author

yadij commented May 19, 2025

PR description: Github merge queues are now able to perform all the required checks and details Squid Project applies to code additions.

When PR #2067 and this PR are both merged we should no longer need Anubis.

Do GitHub merge queues allow immediate merge of a unanimously approved PR while waiting 10 days for a PR approved by one core developer and 48 hours for a PR approved by two core developers?

The 48hr limit is being added by PR #2067 quoted as the other requirements before Anubis was affected.

Anubis does not follow the 10-day criteria in the Squid Project process. Removing Anubis will not affect the need to manually intervene when it is relevant.

Do GitHub merge queues use PR title/description as the commit message and validate its formatting like Anubis does?

FTR; The formatting requirement existed only because we were dumping the commit message details into the website changesets/*.patch files and generating HTML page reports based on those. That entire system was removed, so we no longer have a need to restrict the PR formatting.

(If you want to remove it and the associated S-filed-description labels right now feel free to do so. It would be useful for PRs quoting error messages etc. Out of scope here though.)

I believe the PR title and description are used. I am not aware of whether github use the default git --squash formatting to generate a suffix on the blurb. That said, if it did that would not be an issue for us to adjust to.

These two questions do not enumerate all useful Anubis features. A comprehensive comparison of features should be performed (and these questions can be used as a starting point).

The above statement implies a lot of things, most of which are false. Most notably, it implies that a comparison has not taken place. It has, you have just not been privy to it. (also, removal of Anubis is explicitly not in the scope of this PR).

The relevant analysis of Anubis vs github are disclosed in the PR description. Remainder of the analysis is out of scope.

Overall, Anubis does a lot more than creating a staged commit (for "slow tests").

Strawman. No claim has been made that it does.

Also, GitHub probably uses different metadata (commit message and authors) for the staged commit.

It uses git --squash or git rebase, whichever is configured for the branch merging style.

FWIW, The Anubis "auto" branch is a poor replication of the github merge queue. Anubis suffers from design limitations (or lack of implementation) that imposes the existence of that "auto" branch, fails to include tags and proper authorship attributions without special PR juggling - most of which are unexpected surprise to casual contributors.

If we replace Anubis with GitHub merge queue, we will lose several features and protections that we frequently use.

Please detail what those are, I am not seeing any of particular importance.

FTR; Removal of Anubis is not within the scope of this PR. I simply mentioned that this PR leads us towards that possibility.

@rousskov
Copy link
Contributor

rousskov commented May 19, 2025

PR description: Github merge queues are now able to perform all the required checks and details Squid Project applies to code additions.

When PR #2067 and this PR are both merged we should no longer need Anubis.

Do GitHub merge queues allow immediate merge of a unanimously approved PR while waiting 10 days for a PR approved by one core developer and 48 hours for a PR approved by two core developers?

The 48hr limit is being added by PR #2067 quoted as the other requirements before Anubis was affected.

AFAICT, even with reviewed PR 2067 changes, the answer would still be "no", because that PR does not take the number of votes into account.

Anubis does not follow the 10-day criteria in the Squid Project process.

It does (via config::voting_delay_max that Squid Project sets to 10 days). We have discussed this in the past. Even if one (mis)interprets "Squid Project process" as (roughly speaking) "automatically merge a new PR after 10 days of silence", there is still a 10-day wait in that interpretation. Can Github merge queues be configured to wait 10 days when dealing with a PR approved by one core developer? My quick reading of GitHub merge queues documentation suggests a "no" answer, but I may have easily missed something!

Do GitHub merge queues use PR title/description as the commit message and validate its formatting like Anubis does?

FTR; The formatting requirement existed only because we were dumping the commit message details into the website changesets/*.patch files and generating HTML page reports based on those. That entire system was removed, so we no longer have a need to restrict the PR formatting.

I disagree that we no longer have a need to restrict commit message encoding and formatting. We can discuss/adjust specifics, but some restrictions remain useful regardless of changes in how we publish patches.

I believe the PR title and description are used.

I doubt that GitHub uses PR description because GitHub does not use PR description as the default commit message that GItHub proposes when we manually squash-merge a PR. BTW, if you have not tested how GitHub merge queues deal with multi-commit PRs, please do so (in your/test repository).

These two questions do not enumerate all useful Anubis features. A comprehensive comparison of features should be performed (and these questions can be used as a starting point).

The above statement ... implies that a comparison has not taken place.

There is no such implication. The statement does imply that (at the very least) the existence of a comprehensive comparison ought to be disclosed to become relevant to this discussion!

The relevant analysis of Anubis vs github are disclosed in the PR description. Remainder of the analysis is out of scope.

I disagree on both counts.

Also, GitHub probably uses different metadata (commit message and authors) for the staged commit.

It uses git --squash or git rebase, whichever is configured for the branch merging style.

git commands do not know anything about GitHub PRs, so they cannot use GitHub PR title/description automatically. If GitHub does not supply PR title/description to those git commands, then the staged commit generated by GitHub will have wrong commit message body (at least).

FWIW, The Anubis "auto" branch is a poor replication of the github merge queue. Anubis suffers from design limitations (or lack of implementation) that imposes the existence of that "auto" branch, fails to include tags and proper authorship attributions without special PR juggling - most of which are unexpected surprise to casual contributors.

FWIW, I do not know what "tags and attributions" Anubis "fails to include". I would not call Anubis a "replication of github merge queue" because Anubis predates that GitHub feature, but Anubis should be able to replicate everything GitHub is doing now as far as staged commit itself is concerned. If any relevant Anubis changes are needed, I am happy to collaborate on introducing them. BTW, GitHub merge queues use the equivalent of Anubis "auto" branch; the concept of a staging branch is not specific to Anubis.

If we replace Anubis with GitHub merge queue, we will lose several features and protections that we frequently use.

Please detail what those are, I am not seeing any of particular importance.

I have already mentioned several specific red flags in my comments, including support for various voting timeouts (i.e. immediate/unanimous, fast track, and slow burner), PR title checks, and staged commit message body generation. They are important IMO. I do not think that a comprehensive analysis (shared with Squid Project) should be my responsibility in this case, but I am willing to (continue to) assist.

P.S. To avoid misunderstanding, I do support the removal of Anubis once/if GitHub provides sufficient built-in functionality to replace that bot! AFAICT, GitHub has not reached that milestone yet.

@yadij
Copy link
Contributor Author

yadij commented Aug 30, 2025

P.S. To avoid misunderstanding, I do support the removal of Anubis once/if GitHub provides sufficient built-in functionality to replace that bot! AFAICT, GitHub has not reached that milestone yet.

I beg to differ.

  • Github merge-queue implements the same merge process as Anubis:

for each accepted PR;

  • re-test PR on top of latest master,
  • if passed; apply to master
  • if failed;
    • clear the merge approval, and
    • update PR with test result details
  • Github actions are able to replicate the Squid-specific voting process requirements we have for PR commits. That is being implemented in PR CI: Enforce Squid Project review process voting #2067.

  • PR title and description line limits are no longer necessary since recent v7 release process changes.

    • if we want a specific limit for any reason we can add github action to test and "fail" on arbitrary PR details like that.

AFAIK that is all Anubis is supposed to be doing for Squid project.

@yadij yadij dismissed rousskov’s stale review December 18, 2025 05:48

Issues resolved.

@yadij
Copy link
Contributor Author

yadij commented Dec 18, 2025

Clearing this up for merge as technical issues were resolved months ago. The remaining discussion regarding whether to keep using Anubis is not related to merging this PR.

@yadij yadij added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels backport-to-v7 maintainer has approved these changes for v7 backporting labels Dec 18, 2025
squid-anubis pushed a commit that referenced this pull request Dec 18, 2025
Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Github merge queues are now able to perform all the required
checks and details Squid Project applies to code additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Dec 18, 2025
@squid-anubis squid-anubis added M-merged https://github.com/measurement-factory/anubis#pull-request-labels and removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Dec 18, 2025
@yadij yadij deleted the ci-merge-queue branch December 18, 2025 09:43
squidadm pushed a commit to squidadm/squid that referenced this pull request Dec 18, 2025
Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Github merge queues are now able to perform all the required
checks and details Squid Project applies to code additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project.
github-merge-queue bot pushed a commit that referenced this pull request Dec 18, 2025
Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Github merge queues are now able to perform all the required
checks and details Squid Project applies to code additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project.
@squidadm squidadm removed the backport-to-v7 maintainer has approved these changes for v7 backporting label Dec 18, 2025
@squidadm
Copy link
Collaborator

queued for backport to v7

rousskov added a commit that referenced this pull request Dec 18, 2025
@rousskov
Copy link
Contributor

rousskov commented Dec 18, 2025

yadij yadij dismissed rousskov’s stale review 8 hours ago

@yadij, you do not have the right to dismiss negative votes of other core developers. Doing so violates fundamental Project protections, hurting the Project on multiple levels. I have asked you to stop doing that on multiple occasions. I have reverted the corresponding master branch commit 14f4cb5.

Let's discuss how to move forward from here. I am opening a thread on the Board mailing list.

@yadij yadij restored the ci-merge-queue branch December 26, 2025 04:08
@yadij yadij removed the M-merged https://github.com/measurement-factory/anubis#pull-request-labels label Dec 30, 2025
@yadij yadij reopened this Dec 30, 2025
@squid-anubis squid-anubis added the M-merged https://github.com/measurement-factory/anubis#pull-request-labels label Dec 30, 2025
yadij added a commit to squidadm/squid that referenced this pull request Jan 2, 2026
Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Github merge queues are now able to perform all the required
checks and details Squid Project applies to code additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project.
squidadm pushed a commit to squidadm/squid that referenced this pull request Jan 2, 2026
Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Github merge queues are now able to perform all the required
checks and details Squid Project applies to code additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project.
kinkie added a commit to squidadm/squid that referenced this pull request Jan 2, 2026
squidadm pushed a commit to squidadm/squid that referenced this pull request Jan 3, 2026
Add the workflow piece(s) to make github run our "slow" tests
only on merge queue entries.

Github merge queues are now able to perform all the required
checks and details Squid Project applies to code additions.

The manual "Add to merge queue" button on github UI is an
identical procedural step replacing the current manual
"M-cleared-for-merge" label.

The "Add to merge queue" is not available to PRs until they pass
review approval and the "quick" code tests. The same criteria
Anubis uses to determine whether to start these "slow" tests.

For now Anubis and 'auto' branch are left as-is. They can be
dropped later when the merge queue is formally accepted for
use by Squid Project.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

M-merged https://github.com/measurement-factory/anubis#pull-request-labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants