Conversation
ac3d6ae to
7bf9e8e
Compare
|
@tagliala thank you. I prefer to use npm all where possible. Not pnpm. Let's stick with the defaults. That should work better with cssbundling-rails as well. I had to use yarn in ActiveAdmin for a feature for local development. That may or may not be needed but we can review that there. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #949 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 14 14
Lines 106 106
=========================================
Hits 106 106 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I think the one reason I went with yarn at least for the Procfile was that it had built in file watching, npm does not. |
7bf9e8e to
5da2bf0
Compare
f55de24 to
83c0900
Compare
83c0900 to
28ed200
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the app’s frontend dependency management from Yarn to npm, updating development/CI workflows and Dependabot settings accordingly.
Changes:
- Replace Yarn usage with npm (
yarn.lockremoved,package-lock.jsonadded, docs and Procfile updated). - Update GitHub Actions workflow to set up Node from
.node-versionwith npm caching. - Adjust Dependabot configuration (adds cooldown and updates npm ecosystem settings).
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Removed Yarn lockfile as part of migration to npm. |
package-lock.json |
Added npm lockfile to pin JS dependencies under npm. |
package.json |
Removes Yarn packageManager metadata; retains build script(s). |
README.md |
Updates setup instructions to use npm install instead of yarn install. |
Procfile.dev |
Switches CSS watcher process to npm run build:css. |
.npmrc |
Adds npm configuration (notably install-script behavior). |
.node-version |
Pins Node version for local dev/CI (used by workflow). |
.github/workflows/tests.yml |
Adds Node setup step using .node-version and npm cache. |
.github/dependabot.yml |
Adds cooldown settings and aligns ecosystems (bundler/npm/github-actions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Additionally: - Add cooldown to dependabot configuration - Remove tailwind ignore - Add `.node-version` for parity with `.ruby-version` - Add reference to mise tool version manager
37fc7ca to
47d4cb4
Compare
|
@javierjulio migrated to |
| @@ -0,0 +1 @@ | |||
| lts/krypton | |||
There was a problem hiding this comment.
I prefer we are explicit with the version number so we should use 24.14.1. Follows the same approach as Ruby with an explicit version.
| lts/krypton | |
| 24.14.1 |
| @@ -0,0 +1 @@ | |||
| ignore-scripts=true | |||
There was a problem hiding this comment.
Is it necessary to include this? I don't want to break to watch commands for local development for this app. I understand the concern with security but I don't think it's worth it for a demo app. Is there an alternative without breaking the watch commands?
There was a problem hiding this comment.
The solution would be pnpm with explicit post-install allowance, but then I guess that the supply chain attacks will be directed to such packages
There was a problem hiding this comment.
How can I test that watch does not work without post-install?
Additionally, add cooldown to dependabot configuration