Skip to content
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

Add "checkgenerate" make target to CI #385

Merged
merged 4 commits into from
Apr 18, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
dist/
.idea/
VERSION
.tmp/
23 changes: 21 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
dev_build_version=$(shell git describe --tags --always --dirty)

export PATH := $(shell pwd)/.tmp/protoc/bin:$(PATH)

# TODO: run golint and errcheck, but only to catch *new* violations and
# decide whether to change code or not (e.g. we need to be able to whitelist
# violations already in the code). They can be useful to catch errors, but
# they are just too noisy to be a requirement for a CI -- we don't even *want*
# to fix some of the things they consider to be violations.
.PHONY: ci
ci: deps checkgofmt vet staticcheck ineffassign predeclared test
ci: deps checkgofmt checkgenerate vet staticcheck ineffassign predeclared test

.PHONY: deps
deps:
Expand All @@ -31,6 +33,19 @@ docker:
docker build -t fullstorydev/grpcurl:$(dev_build_version) .
@rm VERSION

.PHONY: generate
generate: .tmp/protoc/bin/protoc
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh. This repo never even had a make generate target!

@go install google.golang.org/protobuf/cmd/protoc-gen-go@a709e31e5d12
@go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.1.0
go generate ./...

.PHONY: checkgenerate
checkgenerate: generate
git status --porcelain
@if [ -n "$$(git status --porcelain)" ]; then \
exit 1; \
fi

.PHONY: checkgofmt
checkgofmt:
gofmt -s -l .
Expand All @@ -44,7 +59,7 @@ vet:

.PHONY: staticcheck
staticcheck:
@go install honnef.co/go/tools/cmd/staticcheck@v0.3.3
@go install honnef.co/go/tools/cmd/staticcheck@v0.4.3
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have Go 1.20 installed, and the older version of staticcheck was blowing up when it happened upon 1.20 object files. So I went ahead and updated it.

staticcheck ./...

.PHONY: ineffassign
Expand Down Expand Up @@ -72,3 +87,7 @@ errcheck:
.PHONY: test
test:
go test -race ./...

.tmp/protoc/bin/protoc:
./download_protoc.sh

32 changes: 32 additions & 0 deletions download_protoc.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env bash

set -e

cd $(dirname $0)

PROTOC_VERSION="22.0"
PROTOC_OS="$(uname -s)"
PROTOC_ARCH="$(uname -m)"
case "${PROTOC_OS}" in
Darwin) PROTOC_OS="osx" ;;
Linux) PROTOC_OS="linux" ;;
*)
echo "Invalid value for uname -s: ${PROTOC_OS}" >&2
exit 1
esac

# This is for macs with M1 chips. Precompiled binaries for osx/amd64 are not available for download, so for that case
# we download the x86_64 version instead. This will work as long as rosetta2 is installed.
if [ "$PROTOC_OS" = "osx" ] && [ "$PROTOC_ARCH" = "arm64" ]; then
PROTOC_ARCH="x86_64"
fi

PROTOC="${PWD}/.tmp/protoc/bin/protoc"

if [[ "$(${PROTOC} --version 2>/dev/null)" != "libprotoc 3.${PROTOC_VERSION}" ]]; then
rm -rf ./.tmp/protoc
mkdir -p .tmp/protoc
curl -L "https://github.com/google/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-${PROTOC_OS}-${PROTOC_ARCH}.zip" > .tmp/protoc/protoc.zip
cd ./.tmp/protoc && unzip protoc.zip && cd ..
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cd ./.tmp/protoc && unzip protoc.zip && cd ..
cd ./.tmp/protoc && unzip protoc.zip && cd ../..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorporated same changes in this PR as from the grpcui one. So this is using pushd/popd now.

fi

1 change: 1 addition & 0 deletions internal/testing/cmd/bankdemo/bank.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

// bankServer implements the Bank gRPC service.
type bankServer struct {
UnimplementedBankServer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually super hate this. 😠

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it stinks. But that's how the latest version of grpc-go works -- this is required and there's a compile error without it. 🤷

allAccounts *accounts
}

Expand Down
Loading