-
Notifications
You must be signed in to change notification settings - Fork 48.3k
[ci] Make maintainer check always remote #32727
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
Conversation
To prevent local modification of the MAINTAINERS file we now always fetch from `main` instead.
facebook/react#32727 removes the `is_remote` param.
Comparing: 2d40460...760c59f Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
@@ -30,41 +26,27 @@ jobs: | |||
outputs: | |||
is_core_team: ${{ steps.check_if_actor_is_maintainer.outputs.result }} | |||
steps: | |||
- uses: actions/checkout@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@poteto have you thought about using action/checkout
to checkout the main branch? It has options for selecting the repo and branch, so it could be:
steps:
- uses action/checkout@v4
- with:
ref: main
repository: facebook/react
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it's a good idea, from what I read it's not a good idea to checkout any code from something triggered via pull_request_target
since it has write access to the target repo and access to secrets.
Always fetching it from main
seems like the safer choice, albeit less convenient
facebook/react#32727 removes the `is_remote` param.
To prevent local modification of the MAINTAINERS file we now always fetch from
main
instead.