ci: streamline shared setup action#379
Conversation
📝 WalkthroughWalkthroughThe changes refactor the GitHub Actions setup composite action to use built-in npm caching from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/actions/setup/action.yml (1)
39-56: Consider deduplicatingapt-get update.Both "Install Moddable" (line 42) and "Install ESP32" (line 53) run
sudo apt-get updatewhen the cache is not hit. Since these steps run sequentially under the same condition, the second call is redundant.♻️ Suggested refactor: move apt-get update to a separate step or remove the duplicate
- name: Install Moddable if: steps.restore-cache-moddable.outputs.cache-hit != 'true' run: | - sudo apt-get update if [ -z "${{ inputs.target-branch }}" ]; then npm run setup else npm run setup -- --target-branch ${{ inputs.target-branch }} fi working-directory: ./firmware shell: bash - name: Install ESP32 if: steps.restore-cache-moddable.outputs.cache-hit != 'true' run: | - sudo apt-get update + sudo apt-get update # Only needed here for ESP32 dependencies source $HOME/.local/share/xs-dev-export.sh && npm run setup -- --device=esp32 working-directory: ./firmware shell: bashAlternatively, add a dedicated "Update apt packages" step before both install steps:
- name: Update apt packages if: steps.restore-cache-moddable.outputs.cache-hit != 'true' run: sudo apt-get update shell: bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/setup/action.yml around lines 39 - 56, The two steps "Install Moddable" and "Install ESP32" both run sudo apt-get update under the same condition (steps.restore-cache-moddable.outputs.cache-hit != 'true'); remove the duplicate by extracting apt-get update into a separate step (e.g., "Update apt packages") that uses the same if condition and runs before both "Install Moddable" and "Install ESP32", then delete the apt-get update line from the bodies of the Install Moddable and Install ESP32 steps so they only run the package-specific commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/actions/setup/action.yml:
- Around line 39-56: The two steps "Install Moddable" and "Install ESP32" both
run sudo apt-get update under the same condition
(steps.restore-cache-moddable.outputs.cache-hit != 'true'); remove the duplicate
by extracting apt-get update into a separate step (e.g., "Update apt packages")
that uses the same if condition and runs before both "Install Moddable" and
"Install ESP32", then delete the apt-get update line from the bodies of the
Install Moddable and Install ESP32 steps so they only run the package-specific
commands.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a13630a-344a-493e-b4f9-0c7f664431e2
📒 Files selected for processing (1)
.github/actions/setup/action.yml
1 participant
Cleans up the shared GitHub Actions setup action by using setup-node npm caching and clearer Moddable cache step naming.
Summary by CodeRabbit