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
+23 -2
2 changed files with 23 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.gitea/workflows/lint.yaml
+1 -1
View File
@@ -14,5 +14,5 @@ jobs:
- run: pip install pre-commit - run: pip install pre-commit
shell: bash shell: bash
- name: Pre Commit - name: Pre Commit
run: SKIP=no-commit-to-branch pre-commit run -a run: SKIP=no-commit-to-branch pre-commit run -a --hook-stage pre-commit --hook-stage manual
shell: bash shell: bash
.pre-commit-config.yaml
+22 -1
View File
@@ -1,3 +1,4 @@
default_stages: [pre-commit, 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
@@ -15,6 +16,7 @@ repos:
args: [--autofix, --no-sort-keys, --no-ensure-ascii] args: [--autofix, --no-sort-keys, --no-ensure-ascii]
- id: check-merge-conflict - id: check-merge-conflict
- id: no-commit-to-branch - id: no-commit-to-branch
stages: [pre-commit]
- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks - repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
rev: v2.16.0 rev: v2.16.0
@@ -33,14 +35,33 @@ repos:
hooks: hooks:
- id: check-renovate - id: check-renovate
- id: check-github-actions - id: check-github-actions
stages: [manual]
- id: check-github-workflows - id: check-github-workflows
stages: [manual]
- repo: https://github.com/pre-commit/pre-commit-hooks - repo: https://github.com/pre-commit/pre-commit-hooks
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
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.
hooks:
- id: build
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.
name: rebuild-distribution
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).
entry: npm run build
language: system
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.
files: ^(index\.js|monitor\.js|package\.json|package-lock\.json)$
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`.
pass_filenames: false
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.
stages: [pre-commit]
- id: add-dist
name: stage-distribution
entry: git add dist/
language: system
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.
files: ^(index\.js|monitor\.js|package\.json|package-lock\.json)$
Philipp marked this conversation as resolved
AutoReview commented 2026-05-16 16:48:06 +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.
pass_filenames: false
stages: [pre-commit]