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

ENH: Trigger PR labeler workflows from forks #2438

Conversation

jhlegarreta
Copy link
Member

Change the GitHub webhook event to a pull_request_target event to
trigger the PR labeler workflows from forks.

See documentation:
https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request_target
and
https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/

PR Checklist

@jhlegarreta jhlegarreta added the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Mar 24, 2021
@jhlegarreta
Copy link
Member Author

jhlegarreta commented Mar 24, 2021

Let's see if this makes the trick.

If the events are triggered too frequently, once that is verified, we may want to restrict them to the [opened, edited] types in a future PR.

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

It would be a great improvement if it worked.

@dzenanz
Copy link
Member

dzenanz commented Mar 24, 2021

If this works, will it allows us to also apply clang-format on PRs from forks?

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Mar 24, 2021

If this works, will it allows us to also apply clang-format on PRs from forks?

Not 100% sure how the commands throughout the comments on a PR work, but chances are that it might.

Edit: not sure if the current clang-format command is triggered using the kwrobot-1 actions, but this action seems to have the ability to trigger such processes.

@dzenanz dzenanz merged commit 3afcc78 into InsightSoftwareConsortium:master Mar 24, 2021
@jhlegarreta jhlegarreta deleted the TriggerPRLabelerWorkflowFromForks branch March 24, 2021 20:50
@jhlegarreta
Copy link
Member Author

😭 The Actions failed:
https://github.com/InsightSoftwareConsortium/ITK/runs/2188115680?check_suite_focus=true

The webhook event is not supported yet:
srvaroa/labeler#13

😔.

@dzenanz
Copy link
Member

dzenanz commented Mar 24, 2021

Should we revert this PR?

@jhlegarreta
Copy link
Member Author

jhlegarreta commented Mar 24, 2021

Should we revert this PR?

Not sure; the workflow does not show the error in the PR/marks the check as passing, and hence it will not be considered as an unsuccessful check when merging the PRs (somewhat similar to what it was before).

Maybe we could track it as an issue? But even then, we purely depend on the author of the labeler action or someone else (I think I might not have the bandwidth to learn go and to try at this time) contributing provided that the feature gets merged into a release in google's go-github repository.

We could ask to the google folks if they will merge it, or propose the labeler action repo owner to switch to track the go-github master branch (the latter being probably not desired).

Edit: another option is to use other labeler actions; I have not found any doing both tasks (labeling according to the PR title and the changed files), but there are some that do one or the other, so we would have two workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants