Skip to content

Commit

Permalink
pre-commit hook for formatting backend code (hashicorp#21682)
Browse files Browse the repository at this point in the history
* Add backend format linting to pre-commit hook

By taking a slight penalty with each commit, we can ensure that
contributors follow the format behavior by default (if they run hooks),
making accidental PRs without proper formatting less likely.
Additionally, fix gofmtcheck to align with the Makefile, fixing the
corresponding fmtcheck target for use with the hook.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Fix formatting errors

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
  • Loading branch information
cipherboy committed Jul 10, 2023
1 parent 3bf1299 commit b0fb3b1
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 16 deletions.
11 changes: 10 additions & 1 deletion .hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ block() {

# Add all check functions to this space separated list.
# They are executed in this order (see end of file).
CHECKS="ui_lint"
CHECKS="ui_lint backend_lint"

# Run ui linter if changes in that dir detected.
ui_lint() {
Expand All @@ -60,6 +60,15 @@ ui_lint() {
$LINTER || block "UI lint failed"
}

backend_lint() {
# Silently succeed if no changes staged for Go code files.
if git diff --name-only --cached --exit-code -- '*.go'; then
return 0
fi

./scripts/gofmtcheck.sh || block "Backend linting failed; run 'make fmt' to fix."
}

for CHECK in $CHECKS; do
# Force each check into a subshell to avoid crosstalk.
( $CHECK ) || exit $?
Expand Down
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,7 @@ proto: bootstrap
protoc-go-inject-tag -input=./helper/identity/mfa/types.pb.go

fmtcheck:
@true
#@sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'"
@sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'"

fmt: ci-bootstrap
find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs go run mvdan.cc/gofumpt -w
Expand Down
20 changes: 10 additions & 10 deletions builtin/logical/pki/path_acme_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1021,23 +1021,23 @@ func TestACMESubjectFieldsAndExtensionsIgnored(t *testing.T) {
acmeClient := getAcmeClientForCluster(t, cluster, directory, nil)
cr := &x509.CertificateRequest{
Subject: pkix.Name{CommonName: domains[0], OrganizationalUnit: []string{"DadgarCorp IT"}},
DNSNames: domains,
}
DNSNames: domains,
}
cert := doACMEForCSRWithDNS(t, dns, acmeClient, domains, cr)
t.Logf("Got certificate: %v", cert)
require.Empty(t, cert.Subject.OrganizationalUnit)

// Use the default sign-verbatim policy and ensure extension does not get set.
domains = []string{"no-ext.dadgarcorp.com"}
// Use the default sign-verbatim policy and ensure extension does not get set.
domains = []string{"no-ext.dadgarcorp.com"}
extension, err := certutil.CreateDeltaCRLIndicatorExt(12345)
require.NoError(t, err)
cr = &x509.CertificateRequest{
Subject: pkix.Name{CommonName: domains[0]},
DNSNames: domains,
cr = &x509.CertificateRequest{
Subject: pkix.Name{CommonName: domains[0]},
DNSNames: domains,
ExtraExtensions: []pkix.Extension{extension},
}
cert = doACMEForCSRWithDNS(t, dns, acmeClient, domains, cr)
t.Logf("Got certificate: %v", cert)
}
cert = doACMEForCSRWithDNS(t, dns, acmeClient, domains, cr)
t.Logf("Got certificate: %v", cert)
for _, ext := range cert.Extensions {
require.False(t, ext.Id.Equal(certutil.DeltaCRLIndicatorOID))
}
Expand Down
6 changes: 3 additions & 3 deletions scripts/gofmtcheck.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

echo "==> Checking that code complies with gofmt requirements..."

gofmt_files=$(gofmt -l `find . -name '*.go' | grep -v vendor`)
if [[ -n ${gofmt_files} ]]; then
echo 'gofmt needs running on the following files:'
gofmt_files="$(find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs go run mvdan.cc/gofumpt -l)"
if [[ -n "${gofmt_files}" ]]; then
echo 'gofumpt needs running on the following files:'
echo "${gofmt_files}"
echo "You can use the command: \`make fmt\` to reformat code."
exit 1
Expand Down

0 comments on commit b0fb3b1

Please sign in to comment.