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

Don't allow custonname which renames Id to ID #7033

Closed
4 tasks
amaury1093 opened this issue Aug 13, 2020 · 0 comments · Fixed by #7032
Closed
4 tasks

Don't allow custonname which renames Id to ID #7033

amaury1093 opened this issue Aug 13, 2020 · 0 comments · Fixed by #7032

Comments

@amaury1093
Copy link
Contributor

amaury1093 commented Aug 13, 2020

Summary

In proto files, don't allow customname which renames Id to ID.

Rationale

In our proto files, we sometimes use gogoproto's customname tag to rename fields, e.g. ClientId to ClientID, to fit what the Go linter recommends. In the generated *.pb.go files, we indeed see fields with ...ID (capitalized).

In #6918, we are adding grpc-gateway, who generates other files *.pb.gw.go. However, grpc-gateway ignores gogoproto's customname tag, and outputs fields with ...Id (PascalCase). See here for an example.

The Go compiler then obviously complains.

Potential solutions

  1. Create a PR on grpc-gateway, to make them take into account gogoproto.
  2. Add some plugins/hacks in our ./scripts to manually change Id->ID inside grpc-gateway-generated files *.pb.gw.go.
  3. Don't allow customname on Id renaming.

#7032 implements 3.

Notes

  • Most of the changes happen in ibc proto files.
  • You're still free to use ...ID in variable names and elsewhere in the code, so it might create some small inconsistencies in namings.
  • We don't remove all customnames (e.g. Ed25519->ED25519, Url->URL are kept). When generating grpc-gateway files, they didn't seem to interfere. For now we only change ID->Id, as these were the ones that made the compiler shout. But there might be some more in the future.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
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 a pull request may close this issue.

1 participant