chore: add pre-commit hook to run validations #7882
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7882 +/- ##
==========================================
+ Coverage 72.72% 72.77% +0.04%
==========================================
Files 235 235
Lines 35100 35177 +77
==========================================
+ Hits 25528 25600 +72
- Misses 7755 7760 +5
Partials 1817 1817 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zhaohuabing
reviewed
Jan 8, 2026
|
|
||
| This will configure Git to run the following checks before each commit: | ||
|
|
||
| * `make lint` |
Member
There was a problem hiding this comment.
Nit: could we also run make lint.fix-golint
Member
|
Would it be simpler to just add a precommit make target? |
cnvergence
previously approved these changes
Jan 8, 2026
Contributor
|
thanks @syedazeez337 this looks good, can you share how much time this hook/make cmd takes ? want to make sure its not too long to start to annoy developers |
arkodg
reviewed
Jan 11, 2026
tools/make/lint.mk
Outdated
|
|
||
| .PHONY: precommit | ||
| precommit: ## Run all necessary steps to prepare for a commit. | ||
| precommit: lint.fix-golint lint test generate manifests gen-check |
Contributor
There was a problem hiding this comment.
lets not run lint.fix-golint and change code, we can find an alt way to highlight this command to the user
arkodg
reviewed
Jan 11, 2026
.pre-commit-config.yaml
Outdated
| hooks: | ||
| - id: make-precommit | ||
| name: make precommit | ||
| entry: bash -c 'GITHUB_ACTION= make precommit' |
Contributor
There was a problem hiding this comment.
why is GITHUB_ACTION= needed
Author
|
Hi @arkodg
|
syedazeez337
dismissed stale reviews from cnvergence and zhaohuabing
via
6fc32a8
Contributor
|
that feels like a long time imo, should ideally be under 30s, does it take the same amount of time when run again ? |
Author
Pre-commit Component TimingsLinters
Other Tasks
Possible Precommit Configurations
@arkodg What do you think should be kept in the pre-commit hook? |
Add a git pre-commit hook configuration (via pre-commit) that runs essential make targets locally before committing: - make lint - make test - make generate - make manifests - make gen-check This helps catch issues early and speeds up PR reviews by ensuring code passes validations before being pushed. Also updates the Developer Guide with instructions on how to install and use pre-commit. Fixes envoyproxy#7878 Signed-off-by: Azeez Syed <[email protected]>
Add a git pre-commit hook configuration (via pre-commit) that runs essential make targets locally before committing: - make lint.fix-golint - automatically fixes Go lint issues - make lint - verifies no lint issues remain - make test - make generate - make manifests - make gen-check This helps catch issues early and speeds up PR reviews by ensuring code passes validations before being pushed. Also updates the Developer Guide with instructions on how to install and use pre-commit, and adds a 'make precommit' target for convenience. Fixes envoyproxy#7878 Signed-off-by: Azeez Syed <[email protected]>
- Remove lint.fix-golint from precommit target to avoid auto-modifying code
- Remove GITHUB_ACTION= workaround from pre-commit config
- Fix root cause in lint.mk by using ${GITHUB_ACTION:-} to handle unset variable
Signed-off-by: Azeez Syed <[email protected]>
zirain
force-pushed
the
feat/add-pre-commit-hook
branch
from
6fc32a8 to
6938870
Compare
Contributor
|
hey @syedazeez337 thanks for running the tests and sharing this data, will bring it up in the community meeting this week |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
make lint,make test,make generate,make manifests,make gen-checkTest plan
pre-commit run --all-filespassesFixes #7878