Conversation
|
|
cmd/wait-for-github/pr.go
Outdated
| ), | ||
| }, | ||
| &cli.BoolFlag{ | ||
| Name: "stop-on-failed-ci", |
There was a problem hiding this comment.
I think this would be more clear if the flag were named something like continue-on-failed-ci or ignore-failed-ci which would default to false to preserve the existing behavior.
cmd/wait-for-github/pr_test.go
Outdated
| Closed: true, | ||
| CIStatus: github.CIStatusFailed, | ||
| }, | ||
| allowFailedCI: true, |
There was a problem hiding this comment.
Instead of using a test-only field here, I think it makes more sense to directly set the prConfig field for the test scenario:
| allowFailedCI: true, | |
| continueOnFailedCI: false, |
|
Commented for one fix to the README doc and a nit (not blocking so feel free to ignore). Also it looks like the PR title is being flagged by the lint check. I think you can just make it something like "feat: Add --ignore-failed-ci". |
dblinkhorn
left a comment
dblinkhorn
left a comment
There was a problem hiding this comment.
Just noticed the commit before last a binary was added. Once that is removed the rest looks good to go!
Co-authored-by: Doug Blinkhorn <52138617+dblinkhorn@users.noreply.github.com> Signed-off-by: Toni Cárdenas <toni@tcardenas.me>
tcard
force-pushed
the
stop-on-failed-ci
branch
from
8a7ffe3 to
38942c4
Compare
@dblinkhorn Wow, well spotted, I'm not sure how that happened. I amended the commit to remove the binary from history at aa6f542. (A squash would've done that too, but just in case.) |
|
Hey @dblinkhorn, just a reminder about this change. I don't have permissions to merge myself, so, just making sure it doesn't fall through the cracks. |
|
Glad you said something because I didn't even think about that! Thanks! |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
@tcard It looks like this needs a rebase a conflicts resolved. |
3 participants
A failed CI doesn't necessarily mean the PR won't get merged. CI could be retried, or it could be non-blocking.
Add a
--ignore-failed-ciflag to skip the CI check, and rely only on the merged or closed status. Defaults to false for backwards-compatibility.