-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
chore: yarnではなくnpmを使うようにする #198
Conversation
📝 Walkthrough<details>
<summary>📝 Walkthrough</summary>
## Walkthrough
The pull request involves a comprehensive transition from Yarn to npm as the package manager across various workflow configurations and documentation. This includes updates to the CI workflows, external test workflows, and the feed generation workflow, where installation commands and caching settings have been changed to npm. The `.yarnrc.yml` file has been removed, eliminating Yarn-specific configurations. Additionally, the `README.md` and `package.json` files have been updated to reflect the new npm commands, while the `vitest.config.ts` file has had its coverage configuration altered to include files previously excluded from coverage.
## Changes
| File | Change Summary |
|----------------------------------------|---------------------------------------------------------------------------------------------------|
| .github/workflows/ci.yml | Transitioned from Yarn to npm; updated cache settings and installation/linting/testing commands. |
| .github/workflows/external-test.yml | Changed from Yarn to npm; updated cache settings and external test command. |
| .github/workflows/generate-feed.yml | Switched from Yarn to npm; modified cache settings and commands for feed generation and site build. |
| .yarnrc.yml | Deleted file containing Yarn configurations (compression level, global cache, node linker, yarn path). |
| README.md | Updated setup instructions and commands to use npm instead of Yarn; minor formatting adjustments. |
| package.json | Modified build and lint scripts to use npm; removed Yarn version requirement and package manager field. |
| vitest.config.ts | Updated coverage configuration to include previously excluded `.yarn/**` directory. |
## Possibly related PRs
- **#182**: The changes in the CI workflow configuration involve a transition from using Yarn to npm for package management, similar to the main PR's focus on switching from Yarn to npm in the CI workflow.
- **#181**: This PR includes updates to the `package.json` file that relate to the overall package management strategy, echoing the main PR's transition to npm.
</details> Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
README.md (1)
67-85
: Improve command block formattingFor better copy-paste experience and to follow markdown best practices, remove the
$
prefix from commands that don't show output.-$ npm install +npm install -$ npm run feed-generate +npm run feed-generate -$ npm run site-serve +npm run site-serve -$ npm run lint +npm run lint -$ npm run test +npm run test🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
72-72: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
73-73: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
75-75: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
76-76: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
81-81: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
82-82: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
84-84: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
85-85: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
.yarn/releases/yarn-4.5.3.cjs
is excluded by!**/.yarn/**
package-lock.json
is excluded by!**/package-lock.json
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (7)
.github/workflows/ci.yml
(5 hunks).github/workflows/external-test.yml
(1 hunks).github/workflows/generate-feed.yml
(2 hunks).yarnrc.yml
(0 hunks)README.md
(1 hunks)package.json
(2 hunks)vitest.config.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- .yarnrc.yml
✅ Files skipped from review due to trivial changes (1)
- vitest.config.ts
🧰 Additional context used
🪛 Markdownlint (0.35.0)
README.md
67-67: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
72-72: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
75-75: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
81-81: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
82-82: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
84-84: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
85-85: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
🔇 Additional comments (4)
.github/workflows/generate-feed.yml (1)
Line range hint 31-52
: LGTM! Correct npm usage in workflow
The changes follow npm best practices:
- Uses correct cache configuration
- Uses
npm ci
for deterministic installations - Correctly formatted npm run commands
package.json (2)
7-12
: LGTM: Script commands properly migrated to npm
The build and lint scripts have been correctly updated to use npm run
instead of yarn
, maintaining the same functionality.
79-80
: LGTM: Yarn engine requirement properly removed
The engines section has been correctly updated to remove the Yarn version requirement while maintaining the Node.js version requirement.
.github/workflows/ci.yml (1)
20-22
: LGTM: CI workflow successfully migrated to npm
All jobs have been correctly updated to use npm instead of Yarn:
- Cache configuration properly set to 'npm'
- Installation commands changed to 'npm install'
- Script execution commands updated to use 'npm run'
Let's verify the npm cache configuration is working as expected:
Also applies to: 33-35, 46-48, 87-89, 100-105
✅ Verification successful
Migration to npm is correctly implemented across all workflow files
All workflow files (.github/workflows/ci.yml, external-test.yml, and generate-feed.yml) have been properly configured:
- All jobs consistently use
cache: npm
with actions/setup-node@v4 - All package installation commands use either
npm install
ornpm ci
- All script executions use
npm run
- No remaining Yarn configurations found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify npm cache configuration in all workflow files
# Check for consistent npm cache configuration across all workflow files
echo "Checking npm cache configuration in workflow files..."
rg -l 'cache: npm' .github/workflows/
# Verify no remaining Yarn configurations
echo "Checking for any remaining Yarn configurations..."
rg -i 'yarn' .github/workflows/
Length of output: 394
Script:
#!/bin/bash
# Let's verify the actual content of workflow files to ensure proper npm configuration
# Check the content of CI workflow
echo "=== CI Workflow ==="
cat .github/workflows/ci.yml
# Check the content of external test workflow
echo -e "\n=== External Test Workflow ==="
cat .github/workflows/external-test.yml
# Check the content of generate feed workflow
echo -e "\n=== Generate Feed Workflow ==="
cat .github/workflows/generate-feed.yml
Length of output: 4899
0f3578b
to
e299828
Compare
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
README.md (1)
67-67
: Consider removing $ prefix from command examplesThe
$
prefix in command examples can make it harder to copy and paste commands. Consider removing them for better user experience.-$ npm install +npm install -$ npm run feed-generate +npm run feed-generate -$ npm run site-serve +npm run site-serve -$ npm run lint +npm run lint -$ npm run test +npm run testAlso applies to: 73-73, 76-76, 82-82, 85-85
🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
.yarn/releases/yarn-4.5.3.cjs
is excluded by!**/.yarn/**
package-lock.json
is excluded by!**/package-lock.json
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (7)
.github/workflows/ci.yml
(5 hunks).github/workflows/external-test.yml
(1 hunks).github/workflows/generate-feed.yml
(2 hunks).yarnrc.yml
(0 hunks)README.md
(1 hunks)package.json
(2 hunks)vitest.config.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- .yarnrc.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- vitest.config.ts
- .github/workflows/external-test.yml
- .github/workflows/generate-feed.yml
- package.json
🧰 Additional context used
🪛 Markdownlint (0.35.0)
README.md
67-67: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
72-72: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
75-75: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
81-81: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
82-82: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
84-84: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
85-85: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
🔇 Additional comments (4)
README.md (1)
67-85
: Verify Node.js compatibility with npm
The README specifies Node.js >= 20. Let's verify this requirement is appropriate with the new npm-based setup.
✅ Verification successful
Node.js >=20 requirement is appropriate for this project
The Node.js version requirement is justified based on the following findings:
- Modern ECMAScript features (import/export statements, async/await) are used extensively across the codebase
- Key dependencies require recent Node.js versions:
@11ty/eleventy
v3.0.0 (requires Node.js 16+)typescript
v5.2.2 (best supported on Node.js 18+)- Modern packages like
@biomejs/biome
v1.9.3
- Dev dependencies like
@types/node
^22.0.0 indicate targeting of newer Node.js features
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package.json for Node.js engine requirements
jq '.engines.node' package.json
# Check if any dependencies have conflicting Node.js requirements
npm ls 2>/dev/null | grep -i "UNMET PEER DEPENDENCY\|invalid"
Length of output: 90
Script:
#!/bin/bash
# Check for any Node.js version-specific features or syntax in the codebase
rg -l 'import\s+.*\s+from|export\s+.*\{.*\}|async.*await' --type js --type ts
# Check package.json for dependencies and their versions
jq '.dependencies, .devDependencies' package.json
# Look for any build or configuration files that might indicate Node.js version requirements
fd -e config.js -e babel.config.js -e tsconfig.json
Length of output: 1988
🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
72-72: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
75-75: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
81-81: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
82-82: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
84-84: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
85-85: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
.github/workflows/ci.yml (3)
20-21
: LGTM: Correct cache configuration for npm
The cache configuration is correctly set to 'npm' across all Node.js setup steps. This will properly cache the npm dependencies and improve workflow execution time.
Also applies to: 33-34, 46-47, 87-88, 100-101
103-105
: LGTM: Feed generation and site build commands
The feed generation and site build commands have been correctly updated to use npm run syntax.
21-22
: Verify npm ci installation in workflows
The switch to npm ci
is correct for CI environments as it ensures deterministic installations. Let's verify the presence of package-lock.json which is required for npm ci
.
Also applies to: 34-35, 47-48, 88-89, 101-103
✅ Verification successful
npm ci
setup is correctly configured
The verification confirms that:
- package-lock.json exists and is properly tracked in git
- No yarn.lock file is present that could cause conflicts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if package-lock.json exists and is committed
if [ -f "package-lock.json" ]; then
echo "package-lock.json exists"
# Check if it's tracked by git
if git ls-files --error-unmatch package-lock.json >/dev/null 2>&1; then
echo "package-lock.json is tracked by git"
else
echo "WARNING: package-lock.json exists but is not tracked by git"
fi
else
echo "ERROR: package-lock.json not found"
fi
# Check for any yarn.lock file that should be removed
if [ -f "yarn.lock" ]; then
echo "WARNING: yarn.lock still exists and should be removed"
fi
Length of output: 242
1697e73
to
48fd1b9
Compare
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
README.md (1)
67-67
: Consider removing dollar signs from command examplesAccording to markdownlint, it's recommended to remove dollar signs (
$
) from command examples when not showing command output. This improves copy-paste functionality and follows markdown best practices.Apply this change to all command examples:
-$ npm install +npm install -$ npm run feed-generate +npm run feed-generate -$ npm run site-serve +npm run site-serve -$ npm run lint +npm run lint -$ npm run test +npm run testAlso applies to: 73-73, 76-76, 82-82, 85-85
🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
.yarn/releases/yarn-4.5.3.cjs
is excluded by!**/.yarn/**
package-lock.json
is excluded by!**/package-lock.json
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (7)
.github/workflows/ci.yml
(5 hunks).github/workflows/external-test.yml
(1 hunks).github/workflows/generate-feed.yml
(2 hunks).yarnrc.yml
(0 hunks)README.md
(1 hunks)package.json
(2 hunks)vitest.config.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- .yarnrc.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- vitest.config.ts
- .github/workflows/generate-feed.yml
- package.json
- .github/workflows/external-test.yml
- .github/workflows/ci.yml
🧰 Additional context used
🪛 Markdownlint (0.35.0)
README.md
67-67: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
72-72: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
75-75: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
81-81: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
82-82: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
84-84: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
85-85: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
🔇 Additional comments (2)
README.md (2)
67-85
: LGTM: Package manager transition is complete
The README has been successfully updated to reflect the transition from Yarn to npm. All commands have been correctly modified, and the instructions remain clear and consistent.
🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
72-72: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
73-73: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
75-75: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
81-81: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
82-82: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
84-84: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
85-85: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
67-67
: Verify npm commands match package.json scripts
The commands have been correctly updated from Yarn to npm. Let's verify that these commands exist in package.json.
Also applies to: 73-73, 76-76, 82-82, 85-85
✅ Verification successful
All npm commands in README.md are correctly defined in package.json
The verification confirms that all required npm scripts mentioned in the README.md (feed-generate
, site-serve
, lint
, test
) exist in package.json. The package.json actually contains additional related scripts like test-coverage
, test-internal
, and various lint subtasks, which is normal and doesn't affect the README's accuracy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all npm commands mentioned in README.md exist in package.json scripts
# Extract npm commands from README.md and check package.json
echo "Checking for these scripts in package.json:"
echo "- feed-generate"
echo "- site-serve"
echo "- lint"
echo "- test"
# Get scripts section from package.json
cat package.json | jq -r '.scripts | keys[]' | grep -E 'feed-generate|site-serve|lint|test'
Length of output: 424
🧰 Tools
🪛 Markdownlint (0.35.0)
67-67: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
yarnの機能が不要なのでシンプルにnpmにしておく。