-
Notifications
You must be signed in to change notification settings - Fork 144
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
Support annotating fields with field extensions #94
base: main
Are you sure you want to change the base?
Support annotating fields with field extensions #94
Conversation
Interesting, I implemented a similar feature here: |
* usurp using macro * address missed message types and fix Elixir.Protobuf problem * fix spacing
@ulissesalmeida that is very interesting! This pr was intended to simply expose custom field options in the dsl, but we want to swap out the using protobuf macro #87 and use these field options is to cast types like in gogoproto gogo/protobuf@a2aaeaf#diff-c8ef3ed341f269ec9c5f2f199d371b7b. One benefit to the field option vs message option approach is that you don't have to go modify the canonical google protos. Regardless of the implementation though, casting is a extremely useful feature. I agree with your points in #90 and #89, it sucks to keep converting to convenient types in code and would be great if we could configure the compiler to do it for us. Looking through other prs, #85 is actually extremely similar to this one (I should have checked before). @tony612 left a comment on it detailing that he thinks its better to write supporting compilers than to modify this one: #85 (comment). However with type casting I don't know if that would be truly possible. For example the generated type for the struct will be wrong unless the generator is changed to be aware of these field options. @ulissesalmeida I would love to know any thoughts you have about moving forward and @tony612 yours as well. Should specifying the generated elixir type for a field be something protobuf-elixir supports? |
a0ed603
to
13438ac
Compare
@lizziepaquette Thank you for thoughts. I agree with you about having field types annotation too. Make sense 👍 Now, about plugins vs changing the compiler approach, it's a hard decision, I don't know what's the best. Having some plugins supports makes sense, or every time we want to add some extension/feature we need to change the original library. It can be very painful for the maintainers. But these plugins should be able to allow a lot of callbacks/hooks. However, some stuff sounds good having in the core compiler, like a way to annotate your types or automatically transform some types. I'm very new in this protobuf world, then I did was easier for me that was: change the core library(that is in elixir haha) |
This new pr has full fledged type casting, love to hear your thoughts. #100 |
Interesting But I have one question, how a user can extend and add their own types? This approach is very different from mine here: Correct me if I'm wrong, your approach is based on fields right? Meanwhile, my approach is based on messages. I think both approaches can live in the core library, but I'm not sure if the library author will want to adopt either haha. |
No description provided.