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

Move build proto dependencies to separate go.mod #3377

Merged
merged 3 commits into from
Sep 14, 2022

Conversation

alexshtin
Copy link
Member

@alexshtin alexshtin commented Sep 13, 2022

What changed?
Move build proto dependencies to separate go.mod.

Why?
To provide better control over build dependency versions.

How did you test it?
Run locally and Buildkite.

Potential risks
No risks.

Is hotfix candidate?
No.

@alexshtin alexshtin requested a review from a team as a code owner September 13, 2022 19:16
Makefile Outdated
@@ -66,22 +71,17 @@ INTEG_TEST_NDC_ROOT := ./host/ndc
PROTO_ROOT := proto
PROTO_FILES = $(shell find ./$(PROTO_ROOT)/internal -name "*.proto")
PROTO_DIRS = $(sort $(dir $(PROTO_FILES)))
PROTO_IMPORTS := -I=$(PROTO_ROOT)/internal -I=$(PROTO_ROOT)/api -I=$(GOPATH)/src/github.com/temporalio/gogo-protobuf/protobuf
PROTO_IMPORTS := -I=$(PROTO_ROOT)/internal -I=$(PROTO_ROOT)/api -I=$(shell go env GOMODCACHE)/github.com/temporalio/gogo-protobuf@$(GOGO_PROTOBUF_VERSION)/protobuf
Copy link
Member

Choose a reason for hiding this comment

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

I think a better way to do this is to do

go list -f '{{.Dir}}' github.com/temporalio/gogo-protobuf/protobuf

to make that work we need an explicit dependency on github.com/temporalio/gogo-protobuf/protobuf in go.mod, which should be easy, just added it to PINNED_DEPENDENCIES with that version and add an import in some go file so it's used

Makefile Show resolved Hide resolved
@alexshtin alexshtin changed the title Use gogo proto files from GOMODCACHE instead of GOPATH Move build proto dependencies to separate go.mod Sep 13, 2022
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@alexshtin alexshtin merged commit cf75441 into master Sep 14, 2022
@alexshtin alexshtin deleted the feature/use-gogo-proto-cache branch September 14, 2022 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants