-
Notifications
You must be signed in to change notification settings - Fork 320
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
chore: improve grpc tooling and upgrade setup-go to v3 #2553
Conversation
@@ -60,10 +60,9 @@ help: ## Show the available commands | |||
|
|||
|
|||
install-tools: | |||
go install github.com/golang/mock/mockgen@v1.6.0 || \ | |||
GO111MODULE=on go install github.com/golang/mock/mockgen@v1.6.0 |
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.
Backwards compatibility is no longer needed.
Codecov ReportBase: 43.91% // Head: 43.98% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2553 +/- ##
==========================================
+ Coverage 43.91% 43.98% +0.06%
==========================================
Files 187 187
Lines 39082 39082
==========================================
+ Hits 17163 17190 +27
+ Misses 20838 20811 -27
Partials 1081 1081
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
f5fc531
to
be85f9b
Compare
with: | ||
version: '3.x' | ||
- run: make proto | ||
- run: git diff -I '^\/\/\s+-?\s+protoc\s+v' --exit-code ## Ignore protoc version comment |
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.
here is a caveat, it is hard to enforce protoc version is the same in all environments.
I could install the latest proto version in github actions and ubuntu is using a match older version by default.
What I noticed however is that the only difference is only the comment with the version. Thus, there is no real impact on the output if we compile from different protoc versions and if there is we can detect on our CI.
Another option is to use a docker image to generate the protoc files as @fracasula had done in the past.
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.
On CI I don't mind, either way it's fine so whatever it works for you 👍
I like the Docker approach mostly for the Makefile for recipes that we are meant to run locally. Mostly for cases like with protoc
where a go install
isn't enough. I use OpenSuse for example and I have to install protobuf-devel
in order to be able to generate the files. The problem arises if somehow you want to install a different version of protoc
.
…re.proto-grpc-refresh
…er-server into chore.proto-grpc-refresh
Description
This PR is addressing the following issues:
grpc
&proto
files. I have added the command in Makefilegh.neting.cc/golang/protobuf/proto
, which is suppressed bygoogle.golang.org/protobuf
. I've updated all of them.While adding the verification step, I noticed that GOPATH/bin was not added to PATH. Something that setup-go v3 is solving:
setup-go v3 also takes care of caching. Thus I've removed
actions/cache@v2
step and used setup-go v3 in all methods.Notion Ticket
https://www.notion.so/rudderstacks/proto-CI-improvements-24f0127f8e614aabab45cd17c14bbf5d
Security