-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pre-commit and linting #112
Conversation
/test all |
This doesn't add any checks in the CI pipeline right? Someone could try to be sneaky (or accidentally) commit with the |
Right, or they could just not run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as long as we communicate that this is an optional step locally but the pipeline will require it to merge.
# Conflicts: # test/e2e/terraform_ssh_e2e_test.go
/test all |
Signed-off-by: Jeff McCoy <code@jeffm.us>
Signed-off-by: Jeff McCoy <code@jeffm.us>
What
Why
By using pre-commit hooks we can easily and automatically catch simple issues. They run automatically on every
git commit
and we can validate that they ran by running them in the CI pipeline and failing if they report any anomalies.Pay particular attention to the changes in CONTRIBUTING.md so the new prerequisites are well understood.
This first PR adds the
.pre-commit-config
file including the following hooks:check-added-large-files
: Fails if any files over 500kb are added. Is great for keeping repos small. If a bigger file needs to be added you can add it by passing the filename to an exceptions list.check-merge-conflict
: Ensures that there are no orphaned Git merge conflict strings in the codebase. These can appear when a git merge is done incorrectly.detect-aws-credentials
: Detects accidentally added AWS credentials before they make it into the Git history. Huzzah!detect-private-key
: Detects private SSH keys and private certificates before they get accidentally added to the git historyend-of-file-fixer
: Ensures that every file in the codebase ends with a new line, which is a POSIX standard.fix-byte-order-marker
: Strips BOMs which are not recommended when using UTF-8trailing-whitespace
: Strips trailing whitespace (except for markdown since it has special meaning)fix-smartquotes
: Automatically changes smartquotes to dumb quotes. Robots hate smartquotes.go-fmt
: Automatically runsgo fmt
on the codebasegolangci-lint
: Automatically runs a linter on the golang codebase. This one is commented out for now but it works and can be turned on very easily once we fix the linting errors (there aren't that many of them)In the documentation I have said that the CI pipeline requires these checks, but that won't be true at the time of merging this PR. That will come later.
Fixes #74
Related to #75