Conversation
There was a problem hiding this comment.
Pull request overview
GitHub Actions の datapack-linter ワークフローに対して、チェックアウト Action の更新と TypeScript 実行環境(ts-node)の最小設定を調整し、宣言ファイル生成の CI 実行を安定させることを目的とした変更です。
Changes:
actions/checkoutを(Asset リポジトリ側の checkout のみ)v5 に更新- CI 上で生成する
tsconfig.jsonにtypes: ["node"]を追加して Node 型定義を明示
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if: ${{ github.ref == 'refs/heads/master'}} | ||
| run: | | ||
| npm i -g ts-node | ||
| npm i -D @types/node |
There was a problem hiding this comment.
npm i -D @types/node will always install the latest @types/node, which can change independently of the runner’s Node version and cause sudden CI breakages. Consider pinning @types/node to the intended major (e.g., 24.x if you’re standardizing on Node 24) or deriving it from the Node version you set via setup-node.
| npm i -D @types/node | |
| NODE_MAJOR="$(node -p "process.versions.node.split('.')[0]")" | |
| npm i -D "@types/node@${NODE_MAJOR}" |
| - name: Setup minimum typescript code runner | ||
| if: ${{ github.ref == 'refs/heads/master'}} | ||
| run: | | ||
| npm i -g ts-node | ||
| npm i -D @types/node | ||
| echo '{"compilerOptions": {"target": "ESNext"}}' > ./tsconfig.json | ||
| echo '{"compilerOptions": {"target": "ESNext", "types": ["node"]}}' > ./tsconfig.json |
There was a problem hiding this comment.
PR title says Node.js is being updated to 24, but this workflow doesn’t install/pin any Node version (it relies on whatever is preinstalled on ubuntu-latest). If the intent is to run on Node 24, add an explicit actions/setup-node step (or similar) and set the version to 24 so the runtime and @types/node align and the change matches the PR description.
| - name: Checkout Asset2 | ||
| if: ${{ github.ref == 'refs/heads/master' }} | ||
| uses: actions/checkout@v3 | ||
| uses: actions/checkout@v5 | ||
| with: |
There was a problem hiding this comment.
Only the second checkout step was bumped to actions/checkout@v5, but the primary Checkout step at the top of the job is still on @v3. Mixing major versions in the same workflow makes maintenance harder and can leave you on an older runtime/security posture for the main checkout; consider updating the initial checkout to the same major version as well.
No description provided.