-
Notifications
You must be signed in to change notification settings - Fork 117
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
feat(tss): the big swap: tssd -> tofnd #297
Conversation
@@ -65,3 +65,7 @@ prereqs: | |||
.PHONY: generate | |||
generate: | |||
go generate -x ./... | |||
|
|||
.PHONE: tofnd-client | |||
tofnd-client: |
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.
this should be moved to the generate
tag
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.
probably not because it requires the user to have protoc installed and that's nontrivial
|
||
.PHONE: tofnd-client | ||
tofnd-client: | ||
@protoc --go_out=. --go-grpc_out=. --go_opt=paths=source_relative --go-grpc_opt=paths=source_relative x/tss/tofnd/tofnd.proto |
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.
this should go into a //go:generate so it's called every time. To have a nicer user experience we can add a check during make prereqs
if protoc is installed
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.
I'm creating a new issue for this. When we add it to make generate
we need to set up the github action to automatically install protoc, which is not trivial. Nevertheless, this is a step we should take. I already found that I'm generating different files from the proto locally because I have a different protoc-gen-go version installed
Pending successful local ceremony. May need to rebase onto master again before merging.