Screeps/screeps-deploy-action
4
1
Fork 0
Code Issues 1 Pull Requests Actions Packages Releases 4 Activity

chore: configure pre-commit stages and add build hook #83

Closed
Philipp wants to merge 6 commits from feature/pre-commit-build into main
pull from: feature/pre-commit-build
merge into: Screeps:main
Conversation 5 Commits 6 Files Changed 2
+7
1 changed files with 7 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit e5ceb46096 - Show all commits
.pre-commit-config.yaml
+7
View File
2
 @@ -55,3 +55,10 @@ repos:
files: ^(index\.js|monitor\.js|package\.json|package-lock\.json)$ files: ^(index\.js|monitor\.js|package\.json|package-lock\.json)$
AutoReview commented 2026-05-16 16:44:13 +02:00
Review
Copy Link
Copy Source

Since npm run build updates the dist/ directory, these changes won't be included in the current commit unless they are staged.
You might want to change the entry to:
entry: /bin/bash -c "npm run build && git add dist/"
Or ensure that developers are aware they need to stage dist/ if the hook fails (though usually, pre-commit hooks should ideally handle the staging if they modify files).

Since `npm run build` updates the `dist/` directory, these changes won't be included in the current commit unless they are staged. You might want to change the entry to: `entry: /bin/bash -c "npm run build && git add dist/"` Or ensure that developers are aware they need to stage `dist/` if the hook fails (though usually, pre-commit hooks should ideally handle the staging if they modify files).
pass_filenames: false pass_filenames: false
stages: [pre-commit] stages: [pre-commit]
AutoReview commented 2026-05-16 16:47:25 +02:00
Review
Copy Link
Copy Source

The file monitor.js is not present in the repository root. If this is a future file, keep it; otherwise, this entry will never trigger the hook for that specific file. Additionally, the build script in package.json only compiles index.js.

The file `monitor.js` is not present in the repository root. If this is a future file, keep it; otherwise, this entry will never trigger the hook for that specific file. Additionally, the `build` script in `package.json` only compiles `index.js`.
AutoReview commented 2026-05-16 16:50:09 +02:00
Review
Copy Link
Copy Source

monitor.js is included in the file filter but does not exist in the repository. If there are no plans to add this file, it should be removed from the regex.

`monitor.js` is included in the file filter but does not exist in the repository. If there are no plans to add this file, it should be removed from the regex.
- id: add-dist
Philipp marked this conversation as resolved
AutoReview commented 2026-05-16 16:48:06 +02:00
Review
Copy Link
Copy Source

monitor.js does not exist in the repository root. If this is not a planned file, it should be removed from the regex to avoid confusion. Additionally, the build script in package.json currently only processes index.js.

`monitor.js` does not exist in the repository root. If this is not a planned file, it should be removed from the regex to avoid confusion. Additionally, the `build` script in `package.json` currently only processes `index.js`.
name: stage-distribution
AutoReview commented 2026-05-16 16:52:52 +02:00
Review
Copy Link
Copy Source

Same as above, monitor.js is referenced here but is missing from the file tree.

Same as above, `monitor.js` is referenced here but is missing from the file tree.
entry: git add dist/
language: system
files: ^(index\.js|monitor\.js|package\.json|package-lock\.json)$
pass_filenames: false
stages: [pre-commit]
AutoReview commented 2026-05-16 16:47:25 +02:00
Review
Copy Link
Copy Source

This is a good solution to ensure the compiled distribution is always included in the commit that modified the source.

This is a good solution to ensure the compiled distribution is always included in the commit that modified the source.
AutoReview commented 2026-05-16 16:50:09 +02:00
Review
Copy Link
Copy Source

Same here, monitor.js is referenced but does not exist in the file tree.

Same here, `monitor.js` is referenced but does not exist in the file tree.
1