Fix potential github action smells #9478
Conversation
- Avoid jobs without timeouts - Use commit hash instead of tags for action versions - Define permissions for workflows with external actions
|
|
Hi @ceddy4395, Welcome, and thank you for contributing to Remix! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected]. Thanks! - The Remix team |
Add ceddy4395 as a contributor
|
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
| outputs: | ||
| published_packages: ${{ steps.changesets.outputs.publishedPackages }} | ||
| published: ${{ steps.changesets.outputs.published }} | ||
| steps: | ||
| - name: ⬇️ Checkout repo | ||
| uses: actions/checkout@v4 | ||
| uses: actions/checkout@a5ac7e51b41094c92402da3b24376905380afc29 # v4 |
There was a problem hiding this comment.
This action is maintained by Github, there should not be any security/breaking changes issues. By pinning to a specific commit hash, you're limiting yourself from potential improvements to the current major version.
This is the same for pnpm and changesets, both are maintained by their respective organization.
There was a problem hiding this comment.
I agree, I think we're fine keeping the numerical versions for these jobs
| @@ -20,20 +25,21 @@ jobs: | |||
| github.repository == 'remix-run/remix' && | |||
| !contains(github.ref, 'nightly') | |||
| runs-on: ubuntu-latest | |||
| timeout-minutes: 2 | |||
There was a problem hiding this comment.
For this job, I don't think 2 minutes is enough, but core maintainers can confirm.
Releases are probably closely monitored, so I don't think there is a real use case to this limit.
There was a problem hiding this comment.
I agree - a quick peek at past runs shows we've had a few jobs in the 3-5 minute range so I wouldn't go lower than 10-15m here if it's just intended to catch true runaway/hung jobs. Anything under that could result in false positives if github's infra is slowed down.
| @@ -9,6 +9,11 @@ on: | |||
| - "!release-manual" | |||
| - "!release-manual-*" | |||
|
|
|||
| permissions: | |||
| contents: write | |||
| pull-requests: write | |||
There was a problem hiding this comment.
Do you have links to what these are doing versus what the default permissions are? Is this more restrictive? Or just more explicit?
|
Thank you for opening this pull request, and our apologies we haven't gotten around to it yet! With the release of React Router v7 we are sun-setting continued development/maintenance on Remix v2. If you have not already upgraded to React Router v7, we recommend you do so. We've tried to make the upgrade process as smooth as possible with our Future Flags. We are now in the process of cleaning up outdated issues and pull requests to improve the overall hygiene of our repositories. We plan to continue to address 2 types of issues in Remix v2:
If you believe this pull request meets one of those criteria, please respond or create a new pull request so it pops up on our radar (since github notifications may get lost in the noise 😕). For all other issues/changes, ongoing maintenance will be happening in React Router v7, so:
If you have any questions you can always reach out on Discord. Thanks again for providing feedback and helping us make our framework even better! |
Hey! 🙂
I've made the following changes to your workflow:
(These changes are part of a research Study at TU Delft looking at GitHub Action Smells. Find out more)
Closes: #
Testing Strategy: