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

all: refactor .pb.go generation #3451

Merged
merged 1 commit into from
Apr 24, 2020
Merged

all: refactor .pb.go generation #3451

merged 1 commit into from
Apr 24, 2020

Conversation

neild
Copy link
Contributor

@neild neild commented Mar 12, 2020

Replace various //go:generate lines and regenerate.sh scripts with a
single, top-level regenerate.sh that regenerates all .pb.go files.

Placing generation in a single script ensures that all files are
generated with similar parameters. The new regenerate.sh uses the
protoc-gen-go version defined in test/tools/go.mod and automatically
handles new .proto files as they are added.

Do some minor refactoring on .proto files: Every file now has a
go_package option (which will be required by a future version of the
code generator), and file imports are all relative to the repository
root.

Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

LGTM but a couple small comments. Can you please also resolve the file with a conflict? Thanks!

regenerate.sh Outdated Show resolved Hide resolved
regenerate.sh Show resolved Hide resolved
regenerate.sh Outdated Show resolved Hide resolved
@neild
Copy link
Contributor Author

neild commented Apr 23, 2020

Resolved the merge conflict.

Ran regenerate.sh, which also picked up a minor change to balancer/rls/internal/proto/grpc_lookup_v1/rls.pb.go.

@neild neild removed their assignment Apr 23, 2020
@dfawley
Copy link
Member

dfawley commented Apr 23, 2020

Ran regenerate.sh, which also picked up a minor change to
balancer/rls/internal/proto/grpc_lookup_v1/rls.pb.go.

Unfortunately, this now triggers a deprecated check, because one of the fields was marked as deprecated. Can you take a look @easwars?

@easwars
Copy link
Contributor

easwars commented Apr 23, 2020

Ran regenerate.sh, which also picked up a minor change to
balancer/rls/internal/proto/grpc_lookup_v1/rls.pb.go.

Unfortunately, this now triggers a deprecated check, because one of the fields was marked as deprecated. Can you take a look @easwars?

Sent #3562.

Replace various //go:generate lines and regenerate.sh scripts with a
single, top-level regenerate.sh that regenerates all .pb.go files.

Placing generation in a single script ensures that all files are
generated with similar parameters. The new regenerate.sh uses the
protoc-gen-go version defined in test/tools/go.mod and automatically
handles new .proto files as they are added.

Do some minor refactoring on .proto files: Every file now has a
go_package option (which will be required by a future version of the
code generator), and file imports are all relative to the repository
root.
@neild
Copy link
Contributor Author

neild commented Apr 23, 2020

Rebased to after #3562 .

@dfawley dfawley merged commit 15653fe into grpc:master Apr 24, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants