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?
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: