-
Notifications
You must be signed in to change notification settings - Fork 602
CI: enable slow build tests for github merge queue #2068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
For now Anubis and 'auto' branch are left enabled. They can be dropped later.
|
When PR #2067 and this PR are both merged we should no longer need Anubis. |
|
nice! |
There was a problem hiding this 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.
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.
FTR; The formatting requirement existed only because we were dumping the commit message details into the website (If you want to remove it and the associated I believe the PR title and description are used. I am not aware of whether github use the default
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.
Strawman. No claim has been made that it does.
It uses 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.
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. |
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.
It does (via
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 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).
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!
I disagree on both counts.
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.
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. |
I beg to differ.
AFAIK that is all Anubis is supposed to be doing for Squid project. |
|
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. |
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.
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.
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.
|
queued for backport to v7 |
@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. |
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.
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.
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.
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
masterbranch.v7branch is listed for Squid Software Maintainers use on thecurrent release branch.
v6and older branches are deprecated and merge rejectionis a desirable outcome there.