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

Keep protobuf fields on one line #618

Closed
ethanfrey opened this issue May 10, 2019 · 0 comments · Fixed by #666
Closed

Keep protobuf fields on one line #618

ethanfrey opened this issue May 10, 2019 · 0 comments · Fixed by #666
Assignees

Comments

@ethanfrey
Copy link
Contributor

ethanfrey commented May 10, 2019

Is your feature request related to a problem? Please describe.

With the use of prototool format in the Makefile, we are getting code like this in the *.proto files:

orm.VersionedIDRef electorate_ref = 5 [
    (gogoproto.customname) = "ElectorateRef",
    (gogoproto.nullable) = false
  ];

The problem here is that these gogoproto.* directives are custom to go and will choke any compiler for other languages. The current iov-core protobuf build chain was stripping out all directives... but it only works when on one line:

sed -ie 's/ *\[[^]]*\];/;/g' "$outfile"

And multiline sed looks like a nightmare: https://unix.stackexchange.com/a/26290

Describe the solution you'd like

As discussed here: https://github.com/iov-one/weave/pull/616/files#r282859931 please configure prototool format to leave long lines. Then I will validate it compiles well with iov-core scripts (and adjust them if needed)

Describe alternatives you've considered

Dropping sed and using perl for string replacement... but I would really like to have the most supported *.proto files possible.

@husio suggested we just do all pre-processing ourselves and produce one protobuf file per command (eg. bnsd, bcpd, etc) that can easily be imported by iov-core. This would remove a script in iov-core that grab these out and clean them up, and make the interface more portable. (Some related commented in #187 which we just did for docs)

Additional context
Add any other context or screenshots about the feature request here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants