Skip to content
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

Automatically count required staging checks #68

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

eduard-bagdasaryan
Copy link
Contributor

@eduard-bagdasaryan eduard-bagdasaryan commented May 26, 2024

Now Anubis requests the list of required staging checks from GitHub
'stating branch' settings instead of maintaining its own
config::staging_checks parameter. That parameter was problematic because
it was usually out of sync with the actual list of required checks.

Now Anubis requests the list of required staging checks from GitHub
'stating branch' settings instead of maintaining its own
config::staging_checks parameter. That parameter was problematic because
it was usually out of sync with the actutal list of required checks.
With new approach, the list of required staging checks should still be
kept up-to-date. However, it became easier because it must include CI
checks only (GitHub check runs are maintained automatically).
Anubis now expects that the staging branch protection
includes the complete list of required checks.
That list is extracted from GitHub (the same way as for
PR itself) and the size of that list is the actual number
of expected staging checks.
@@ -234,7 +229,7 @@ class StatusChecks

// no more required status changes or additions are expected
final() {
return (this.requiredStatuses.length >= this.expectedStatusCount) &&
return (this.requiredStatuses.length >= this._contextsRequiredByGitHubConfig.length) &&
Copy link
Contributor Author

@eduard-bagdasaryan eduard-bagdasaryan May 28, 2024

Choose a reason for hiding this comment

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

this.requiredStatuses.length should be <= this._contextsRequiredByGitHubConfig.length because this.requiredStatuses gets only those statuses that are among this._contextsRequiredByGitHubConfig. So we can probably assert the '<=' condition here and use '==' instead for the returning condition.

expected to auto-test the staging branch on every change. A check
failure is a PR-specific step failure -- in GitHub terminology, all
staging checks are currently deemed "required". If discovered, any
extra check is considered a configuration problem (not related to any
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now all 'extra' checks will be treated as optional (i.e., the checks that are not 'required').

@@ -1016,10 +1018,13 @@ class PullRequest {
statusChecks.addOptionalStatus(new StatusCheck(st));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_getPrStatuses() and _getStagingStatuses() look almost the same now, except for this dealing with 'copied' statuses (although the diff does not show it clearly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant