chore: configure pre-commit stages and add build hook #83
@@ -14,5 +14,5 @@ jobs:
|
||||
- run: pip install pre-commit
|
||||
shell: bash
|
||||
- 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
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
default_stages: [pre-commit, manual]
|
||||
exclude: ^(dist|node_modules)/
|
||||
repos:
|
||||
- repo: https://github.com/pre-commit/pre-commit-hooks
|
||||
@@ -15,6 +16,7 @@ repos:
|
||||
args: [--autofix, --no-sort-keys, --no-ensure-ascii]
|
||||
- id: check-merge-conflict
|
||||
- id: no-commit-to-branch
|
||||
stages: [pre-commit]
|
||||
|
||||
- repo: https://github.com/macisamuele/language-formatters-pre-commit-hooks
|
||||
rev: v2.16.0
|
||||
@@ -33,14 +35,33 @@ repos:
|
||||
hooks:
|
||||
- id: check-renovate
|
||||
- id: check-github-actions
|
||||
stages: [manual]
|
||||
- id: check-github-workflows
|
||||
stages: [manual]
|
||||
|
||||
- repo: https://github.com/pre-commit/pre-commit-hooks
|
||||
rev: v6.0.0
|
||||
hooks:
|
||||
- id: end-of-file-fixer
|
||||
exclude: (.txt$|.ipynb$|README.md$|readme.mde$)
|
||||
exclude: (.txt$|.ipynb$|README.md$)
|
||||
- id: trailing-whitespace
|
||||
exclude: (.txt$|README.md$)
|
||||
- id: mixed-line-ending
|
||||
args: [--fix=lf]
|
||||
|
||||
- repo: local
|
||||
|
|
||||
hooks:
|
||||
- id: build
|
||||
AutoReview
commented
You've added 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
Since 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
The file 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
`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
`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
Same as above, 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
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
Same here, 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
Same as above, Same as above, `monitor.js` is referenced here but is missing from the file tree.
|
||||
pass_filenames: false
|
||||
stages: [pre-commit]
|
||||
|
||||
monitor.jsis 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.