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

Report PRs with required status failures #12

Closed
wants to merge 2 commits into from
Closed

Conversation

pbrisbin
Copy link
Member

@pbrisbin pbrisbin commented May 14, 2024

Default variables in-script too

19024e8

This makes it easier to test locally. The defaults now match the
defaults in action.yml, except for DRY_RUN, naturally.

Report PRs with required status failures

995ab99

Assuming PRs are left to mergeabot, these failures would never be
visible to the team since the reviewer has been removed and the
assumption is that the PR will pass and be merged eventually.

We raise visibility in 3 ways here:

  1. Report on each PR which required statuses failed, if any
  2. Report at the end the list of such PRs
  3. Set failed and failed-urls outputs for further action

pbrisbin added 2 commits May 14, 2024 14:02
This makes it easier to test locally. The defaults now match the
defaults in `action.yml`, except for `DRY_RUN`, naturally.
Assuming PRs are left to mergeabot, these failures would never be
visible to the team since the reviewer has been removed and the
assumption is that the PR will pass and be merged eventually.

We raise visibility in 3 ways here:

1. Report on each PR which required statuses failed, if any
2. Report at the end the list of such PRs
3. Set `failed` and `failed-urls` outputs for further action
@pbrisbin pbrisbin changed the title pb/fail on failure Report PRs with required status failures May 14, 2024
@pbrisbin pbrisbin requested a review from joelmccracken May 14, 2024 18:23
@pbrisbin pbrisbin marked this pull request as ready for review May 14, 2024 18:23
Comment on lines +97 to +100
gh api \
"/repos/$GH_REPO/branches/$defaultBranch/protection" \
--jq '.required_status_checks.contexts[]' |
sort >"$tmp/required-statuses.txt"
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, so this requires a permission that I'm not sure Actions is able to even request.

That might be a deal-breaker for this whole idea.

Copy link
Member Author

@pbrisbin pbrisbin May 14, 2024

Choose a reason for hiding this comment

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

I think the choices are:

  1. Ask for the list of required statuses as an input, or
  2. Do this if any status checks fail

Choose a reason for hiding this comment

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

Is there some way to attempt a merge, which would be rejected if a required status is failed?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea. I don't know what happens if a bot tries to merge around branch protections, I guess we'd have to test it.

@pbrisbin pbrisbin closed this May 23, 2024
@pbrisbin pbrisbin deleted the pb/fail-on-failure branch May 23, 2024 18:39
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.

2 participants