pre-commit to run fmt.sh (gofmt / gosimple / buildifier)#1818
pre-commit to run fmt.sh (gofmt / gosimple / buildifier)#1818rshriram merged 16 commits intoistio:masterfrom
Conversation
|
/assign @ayj |
|
Thanks. This enforcement is client-side which can be bypassed. Can we add the same thing to istio-presubmit here? |
|
good idea, done. |
Codecov Report
@@ Coverage Diff @@
## master #1818 +/- ##
==========================================
+ Coverage 81.72% 81.73% +<.01%
==========================================
Files 200 203 +3
Lines 20330 20463 +133
==========================================
+ Hits 16615 16725 +110
- Misses 3251 3267 +16
- Partials 464 471 +7
Continue to review full report at Codecov.
|
WORKSPACE
Outdated
|
|
||
| mixer_adapter_repositories() | ||
|
|
||
| go_repository( |
There was a problem hiding this comment.
btw, these things are sometimes auto-imported as part of rules_go.
https://github.com/istio/istio/blob/master/pilot/bin/check.sh#L10
|
@jmuk PR needs rebase |
|
@jmuk thanks for doing this. It seems for some reason fmt.sh is not found, and that is causing the presubmit to fail. |
The previous code causes a failure because of set -e.
|
fixed, presubmit now passes (previous fmt.sh had a bug on checking goimports / buildifier). PTAL. |
|
/lgtm |
|
/assign @sebastienvas |
|
@jmuk PR needs rebase |
|
/lgtm cancel //PR changed after LGTM, removing LGTM. @ayj @guptasu @jmuk @sebastienvas |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: guptasu Assign the PR to them by writing The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:You can indicate your approval by writing |
| # Build | ||
| ${ROOT}/bin/init.sh | ||
|
|
||
| run_or_die_on_change ./bin/fmt.sh |
There was a problem hiding this comment.
Move this to line ~90 before any additional temporary build files are generated. Otherwise things like pilot/proxy/envoy/envoy will show up in git diff and incorrectly fail the build.
|
/lgtm cancel //PR changed after LGTM, removing LGTM. @ayj @guptasu @jmuk @sebastienvas |
* Update the api sha, istio/api#518. * Remove TLS_PERMISSIVE usage.
What this PR does / why we need it:
It's quite easy to forget formatting the files. This enforces to run fmt.sh, which takes care of trivial style changes on go files and BUILD files.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: