Skip to content

Auto-request review for usability when label is applied.#116884

Closed
Ivorforce wants to merge 1 commit into
godotengine:masterfrom
Ivorforce:usability-auto-request
Closed

Auto-request review for usability when label is applied.#116884
Ivorforce wants to merge 1 commit into
godotengine:masterfrom
Ivorforce:usability-auto-request

Conversation

@Ivorforce
Copy link
Copy Markdown
Member

@Ivorforce Ivorforce commented Feb 28, 2026

Suggested by @AdriaandeJongh. Would appreciate more input on usability team if this is desired.

I'm also not sure if this will work since I haven't tested it.
The action depends on https://github.com/kunihiko-t/review-request-action which is niche but also small, so hopefully no risk of regression if it does work.

@Ivorforce Ivorforce added this to the 4.x milestone Feb 28, 2026
@Ivorforce Ivorforce requested a review from a team as a code owner February 28, 2026 11:28
@Ivorforce Ivorforce requested a review from a team February 28, 2026 11:29
Copy link
Copy Markdown
Contributor

@passivestar passivestar left a comment

Choose a reason for hiding this comment

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

Would appreciate more input on usability team if this is desired.

Makes sense to me

@AThousandShips
Copy link
Copy Markdown
Member

Not sure this will work without adding permissions to the token used for CI, which has its own risks

@Ivorforce Ivorforce force-pushed the usability-auto-request branch from efb72c3 to 5eec605 Compare February 28, 2026 11:43
@Ivorforce
Copy link
Copy Markdown
Member Author

Not sure this will work without adding permissions to the token used for CI, which has its own risks

Good point. I changed the ref to a specific commit instead of master, which should mitigate / remove the risks. I've looked at the source code for the ref in question and it looks safe to me, but with @master we'd risk a change in the source repository going live without us knowing.

@AThousandShips
Copy link
Copy Markdown
Member

That's not the risk I mean, I mean that if we add permissions to the runners someone can write a GitHub action to delete all labels for example or close all PRs, as there's no real way for us to control what the token is used for when it's configured as it has to be accessible to this CI action

@AThousandShips
Copy link
Copy Markdown
Member

I think this feature is better implemented as a GitHub app or other background process where we can control access but not expose like this

Copy link
Copy Markdown
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

This won't work, looking closer at the action package it only runs when a PR is opened, it won't run at any other time so this unfortunately won't work

    if (context.payload.action !== 'opened') {
      console.log('No pull request was opened, skipping')
      return
    }

@AThousandShips
Copy link
Copy Markdown
Member

It turns out it applies the action to this specific PR, and it fails, but not for the reasons above I think

@Ivorforce
Copy link
Copy Markdown
Member Author

Right, so it's broken anyway. I guess if we wanted to do this we would need another or a custom action.
Thanks for testing!

@Ivorforce Ivorforce closed this Feb 28, 2026
@Ivorforce Ivorforce deleted the usability-auto-request branch February 28, 2026 13:32
@AThousandShips AThousandShips removed this from the 4.x milestone Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants