Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Companion PR for #8682 #2958

Merged
merged 7 commits into from
May 6, 2021
Merged

Companion PR for #8682 #2958

merged 7 commits into from
May 6, 2021

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Apr 29, 2021

@tomaka tomaka added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. C1-low PR touches the given topic and has a low impact on builders. labels Apr 29, 2021
@tomaka tomaka requested a review from kpp April 29, 2021 13:26
@ghost
Copy link

ghost commented May 6, 2021

Waiting for commit status.

@ghost
Copy link

ghost commented May 6, 2021

Merge aborted: Checks failed for e67f157

@ordian
Copy link
Member

ordian commented May 6, 2021

@joao-paulo-parity simnet check is not required, but the companion merge has failed nevertheless, is this expected?

@ordian ordian merged commit 79b37d3 into master May 6, 2021
@ordian ordian deleted the tka-companion-8682 branch May 6, 2021 14:41
@TriplEight
Copy link
Contributor

@ordian simnet check was supposed to be required (and was), but I had to unmark it bc Simnet was a bit broken. Now it's about to be stabilized and the "required" check will return.

@TriplEight
Copy link
Contributor

But the fact that the bot has "required" checks configured elsewhere is rather odd, though.

@joao-paulo-parity
Copy link
Contributor

joao-paulo-parity commented May 6, 2021

@joao-paulo-parity simnet check is not required, but the companion merge has failed nevertheless, is this expected?

@TriplEight @ordian

The trigger-simnet job does not have allow_failure: true, so it is expected. That is solely what determines if a check can be skipped. We don't consider Github "required status" settings into account for skipping fallible statuses.

trigger-simnet:

@ordian
Copy link
Member

ordian commented May 7, 2021

But the fact that the bot has "required" checks configured elsewhere is rather odd, though.

The trigger-simnet job does not have allow_failure: true, so it is expected. That is solely what determines if a check can be skipped. We don't consider Github "required status" settings into account for skipping fallible statuses.

I don't understand what's the distinction between allow_failure: true and Github "requered status" setting, and will let you judge whether its needs to have allow_failure: true. But please make sure simnet check is ignored by bot until it's proved to be stable (e.g. not spuriously failing for a week).

@cla-bot-2021
Copy link

cla-bot-2021 bot commented May 7, 2021

@ordian No commands could be detected from your comment.

@joao-paulo-parity
Copy link
Contributor

@ordian

#2958 (comment) is some sort of bug, don't mind it

I don't understand what's the distinction between allow_failure: true and Github "requered status" setting

  • allow_failure: true is the job's outcome from Gitlab.

  • Github "required status" are statuses which will block the merge if they are not passing.

If any Gitlab check needs to be ignored temporarily, simply put allow_failure: true in .gitlab-ci.yml for that job. That setting is precisely for brittle/fallible/unstable jobs. It doesn't make sense to me to create custom rules in the bot when that setting is already proper.

@paritytech paritytech deleted a comment from cla-bot-2021 bot May 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants