Add pre-commit hooks #5
Conversation
Signed-off-by: Daniil Klimuk <[email protected]>
|
Verification logs: check for added large files..............................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
detect private key.......................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
mixed line ending........................................................Passed
codespell................................................................Passed
yamllint.................................................................Failed
- hook id: yamllint
- exit code: 1
.pre-commit-config.yaml
51:8 error too many spaces after hyphen (hyphens)
51:5 error wrong indentation: expected 6 but found 4 (indentation)
markdownlint.........................................(no files to check)Skipped
markdownlint-fix.....................................(no files to check)Skipped
ShellCheck v0.9.0....................................(no files to check)SkippedSo, it works and even detected some spells. |
We have some shell files, and I would expect many failures there. Not that we have to fix them right now, but why these are not checked?
We have README.md , at least? |
Reason: ignore "variable appears unsued" beacause scripts in this directory are being sourced by other scripts in the repo, and, therefore, so not use self declared variables. Solutions I have also tried: - adding "# shellcheck source=../include/dts-functions.sh" and "# shellcheck source=../include/dts-environment.sh" to all scripts under path ./scripts/; - adding "# shellcheck source=SCRIPTDIR/../include/dts-functions.sh" and "# shellcheck source=SCRIPTDIR/../include/dts-environment.sh" to all scripts under path ./scripts/; - adding "source-path=./include" and "source-path=SCRIPTDIR/../include" to ".shellcheckrc" as well as via command line arguments and comments in shell scriptfiles; - mixing above solutions in different ways; All above solutions did not work. References: https://www.shellcheck.net/wiki/Directive https://www.shellcheck.net/wiki/SC2034 https://www.shellcheck.net/wiki/SC1090 https://pre-commit.com/#config-args Signed-off-by: Daniil Klimuk <[email protected]>
|
@macpijan, my bad, thanks for noticing. After fixes added in 433483c and 478de5a: λ pre-commit run -a
check for added large files..............................................Passed
check for merge conflicts................................................Passed
check for broken symlinks............................(no files to check)Skipped
detect private key.......................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
mixed line ending........................................................Passed
codespell................................................................Passed
yamllint.................................................................Passed
markdownlint.............................................................Passed
markdownlint-fix.........................................................Passed
ShellCheck v0.9.0........................................................Passed |
|
@DaniilKl I said there is no need to fix all of the shellcheck errors here, but I am very glad you did. |
For list of pre-commit warnings checkout: https://www.shellcheck.net/wiki/ Signed-off-by: Daniil Klimuk <[email protected]>
DaniilKl
force-pushed
the
add-pre-commit
branch
from
478de5a
to
e6d7198
Compare
|
@DaniilKl a little description would not hurt, even after merging |
|
@tlaurion description of what exactly? |
This PR description : tools enabled why and how. You can discard, but PR description can be searched globally. I took personal notes of this because Pre-commit hooks are amazing for linting, typos and syntax validation prior of commit, which limits the number of unneeded commits fixing things ad-hoc under unneeded additional commits. Anyway, thanks for this. This is good example for the rest of the ecosystem and I'll try to replicate in my own projects. |
|
@tlaurion Oh, got it. We have a templates and script for putting up a set of tools and configs in a new repo: https://github.com/3mdeb/hooks/blob/main/pre-commit-init/pre-commit-init.py#L60 Feel free to try to use it and submit issues if not clear. We have selected specific tools and rules we like for specific types of files, and we aim to adopt the same rules across the board. So we could in fact add in the MR description @DaniilKl that this was added using this script. |
Add pre-commit hooks to simplify reviewing and code maintaining. The hooks have been automatically configured using 3mdeb/hooks repository and its
pre-commit-init.pyscript.