-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
AVRO-1524: avro-maven-plugin be able to generate schema from IDL #2568
base: main
Are you sure you want to change the base?
AVRO-1524: avro-maven-plugin be able to generate schema from IDL #2568
Conversation
Thank you for implementing this! The idea is good, but plugin mojos should have one task. Can you please create separate mojos for each of the two new actions? |
Agree that plugin mojos should have single responsibility. But the mojo names confused me. Since #1589, we have:
And the new Mojos might be: |
Looks good, but I'm not a fan of the Given that #1589 hasn't been released yet, what do you think of the following:
|
Thanks for your comment. Just pushed some updates. Please help to take a look if you have time @opwvhk |
Thank @opwvhk for your approval. May I ask how to merge it to main? |
Any chance of getting this feature into a release soon? |
@@ -66,14 +66,18 @@ A Maven plugin is also provided to compile .avdl files. To use it, add something | |||
<executions> | |||
<execution> | |||
<goals> | |||
<goal>idl</goal> | |||
<goal>idl2java</goal> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a user of the Java SDK but this looks like a breaking change.
Old applications that use <goal>idl</goal>
will break ?! I see no goal idl
in the list of available goals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.. I should have noticed that.
That said, there's also work underway (for the Java SDK) to parse IDL schemas using the same code as the standard JSON schemas.
Going forward with this PR, we should thus recreate the old goal. Preferably as near-empty subclass of the new mojo (logging a warning directing users to the new name). As the parameters have not changed, this will ensure backwards compatibility.
Any chance of getting this feature into a release soon? |
What is the purpose of the change
Implement AVRO-1524: update avro-maven-plugin to generate schema from IDL
Verifying this change
This change consists of several parts:
All changes have tests.
Documentation
no)not applicable/ docs /JavaDocs/not documented)