Add a pre-commit linter hook#1624
Conversation
6ca0e54 to
ee42d57
Compare
|
""" WalkthroughA new Git pre-commit hook script is added to run Changes
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
.golangci.yml (1)
8-9: Restore trailing newline to satisfy lintersThe removal of the final newline triggers
YAMLlinterrornew-line-at-end-of-file, and some editors re-add it automatically, causing noisy diffs.- - zz_generated\.go + - zz_generated\.go\n🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: no new line character at the end of file
(new-line-at-end-of-file)
.githooks/pre-commit (1)
4-10:STAGED_FILESvariable is built but never usedYou collect the staged file list only to test for emptiness, then ignore it when invoking the linter. Either:
- Drop the unused pipeline and simply test via
git diff --cached --quiet --diff-filter=ACMR -- '*.go'- Or pass the file list to
golangci-lint run --path-prefix/--files, which speeds up execution on large repos.Cleaning this up avoids needless subshells.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.githooks/pre-commit(1 hunks).golangci.yml(1 hunks)Makefile(3 hunks)go.mod(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.golangci.yml
[error] 9-9: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: run-kms-key-rotate-test
- GitHub Check: cnpg-deployment-test
- GitHub Check: run-core-config-map-tests
- GitHub Check: run-operator-tests
🔇 Additional comments (1)
Makefile (1)
326-333: Nice touch: idempotent Git hooks installationThe
install-hooksrecipe correctly checks for.gitand an existingcore.hooksPath, avoiding noisy output. 👍
c1f4507 to
10af05c
Compare
ffeeef2 to
9397c1d
Compare
f85f9fd to
83d6b8b
Compare
Signed-off-by: Ben <belimele@redhat.com>
83d6b8b to
e008adc
Compare
Explain the changes
.githooksdirectory and configuringcore.hooksPathautomatically upon calls to mostmake <target>runs (in order to ensure consistency across devs and envs)install-hooksMakefile target that sets the Git hooks path to.githooksonly if not already set, and only if the repository is a Git repo.install-hooksa dependency of thegentarget, ensuring hooks are installed for both containerized and non-containerized workflows.Testing Instructions:
go mod vendorto make suregolangci-lintis installed with the right version and in the right pathmake gen,make build,make all)..githooksis set as the hooks path (git config core.hooksPathshould output.githooks).make genand confirm there are no prints related to git hooks in the console.Alternatively, after cloning the repo,
cdinto the repo's root and rungit config core.hooksPath .githooks.Then, follow the instructions from step 3 onwards.
Summary by CodeRabbit