snproj
commented
1 year ago Hi! This sounds like a good idea to me; I also personally prefer the proto comments approach. I can think of some pros and cons to proto comments:
Pros
Compared to build flags, as you say they look easier to maintain and work with. We can:
- Carry Rust-specific information inside proto files without affecting other proto compilers
- Place this information more naturally next to the messages they affect, rather than having to remember to dump them all in one go each time before calling
pb-rs.
Con
On the other hand, one downside I can think of for proto comments is that we have to actually modify the proto file itself. Proto files that at some point have been touched by a user of pb-rs may now be marked with what is essentially noise to users of literally any other compiler (even other Rust proto compilers).
Whether this will be an issue I think will depend on the use case of the proto file itself and whether it's very specific in its circulation (e.g. internally within a company), or if it will be exposed to the public at large. Thoughts?
Unless we can convince others to use the comment convention 👀 🤔
Implementation
Looking at the code, I'm wondering whether it's a good idea to implement this in a more generalized way, so that:
- We might easily add in more keywords later on (like controlling whether a struct is owned perhaps?)
- Other users (even those who've not heard of
pb-rs) will at least be able to tell that these are magic comments that apply to Rust / a certain proto compiler.
What do you think?
On a side note, the addition of EnumField is nice; I was about to do something like that in order to properly implement deprecated fields for enums.
I have two changes on my personal repo for adding attributes to structs. One is to allow custom structs via build flags while the other adds support for magic proto comments that are included in the generated code.
I've found the proto comments to be easier to maintain and nicer to work with. Thoughts on these two approaches? I'll open a PR for adding proto comment support if others like the "API".