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

M-failed-staging-checks ignored when staging checks succeed #38

Open
rousskov opened this issue May 17, 2020 · 2 comments
Open

M-failed-staging-checks ignored when staging checks succeed #38

rousskov opened this issue May 17, 2020 · 2 comments

Comments

@rousskov
Copy link
Contributor

Based on #36 (comment):

According to the the M-failed-staging-checks documentation, Squid PR 629 was mishandled by Anubis: Anubis should have aborted aborted merging despite successful "merged" statuses.

On the other hand, honoring the label may look strange from the user point of view: Seeing only green marks, the user may wonder why the PR was (and is) marked as "failed". Also, discarding a ready-for-immediate-merge staged commit is wasteful. Perhaps we should revise Anubis documentation and code to allow the bot to clear M-failed-staging-checks in such cases?

@rousskov
Copy link
Contributor Author

@eduard-bagdasaryan, assuming #36 is merged, is it possible that M-failed-staging-checks label is set for reasons other than a staging checks failure? I am asking this question to decide which way to go:

  • If the bot only sets M-failed-staging-checks when the staging checks fail, then the bot should clear it when that state is no longer true, but
  • if there are other conditions (e.g., an internal assertions while processing those checks), then we probably should not discard the label that may indicate another serious problem.

@eduard-bagdasaryan
Copy link
Contributor

is it possible that M-failed-staging-checks label is set for reasons other than a staging checks failure?

No, the only reason for applying M-failed-staging-checks now is the obtaining of failed statuses. There are two such places in the code: _processStagingStatuses() and _loadPrState().

f the bot only sets M-failed-staging-checks when the staging checks fail, then the bot should clear it when that state is no longer true

Agreed.

e.g., an internal assertions while processing those checks

In such cases of unexpected errors, M-failed-other should be applied.

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

No branches or pull requests

2 participants