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
+12 -1
1 changed files with 12 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3abc837f78 - Show all commits
.pre-commit-config.yaml
+12 -1
View File
@@ -1,3 +1,4 @@
default_stages: [pre-commit, pre-push, manual]
exclude: ^(dist|node_modules)/ exclude: ^(dist|node_modules)/
repos: repos:
- repo: https://github.com/pre-commit/pre-commit-hooks - repo: https://github.com/pre-commit/pre-commit-hooks
@@ -39,8 +40,18 @@ repos:
rev: v6.0.0 rev: v6.0.0
hooks: hooks:
- id: end-of-file-fixer - id: end-of-file-fixer
exclude: (.txt$|.ipynb$|README.md$|readme.mde$) exclude: (.txt$|.ipynb$|README.md$)
- id: trailing-whitespace - id: trailing-whitespace
exclude: (.txt$|README.md$) exclude: (.txt$|README.md$)
- id: mixed-line-ending - id: mixed-line-ending
args: [--fix=lf] args: [--fix=lf]
- repo: local
hooks:
- id: build
name: rebuild-distribution
AutoReview commented 2026-05-16 16:52:52 +02:00
Review
Copy Link
Copy Source

monitor.js is still included in the regex but does not exist in the repository. As noted in previous reviews, if there are no immediate plans for this file, it's cleaner to remove it.

`monitor.js` is still included in the regex but does not exist in the repository. As noted in previous reviews, if there are no immediate plans for this file, it's cleaner to remove it.
entry: npm run build
language: system
AutoReview commented 2026-05-16 16:44:13 +02:00
Review
Copy Link
Copy Source

You've added monitor.js to the list of files that trigger a rebuild. However, looking at package.json, the build script only targets index.js. If monitor.js is not imported by index.js, changes to it won't be reflected in the dist/ folder via the current build command.

You've added `monitor.js` to the list of files that trigger a rebuild. However, looking at `package.json`, the build script only targets `index.js`. If `monitor.js` is not imported by `index.js`, changes to it won't be reflected in the `dist/` folder via the current build command.
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
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.
1