Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2023-10-25 Thread via GitHub


opwvhk commented on PR #2568:
URL: https://github.com/apache/avro/pull/2568#issuecomment-1778798155

   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?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2023-10-25 Thread via GitHub


databius commented on PR #2568:
URL: https://github.com/apache/avro/pull/2568#issuecomment-1779662243

   > 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 https://github.com/apache/avro/pull/1589, we have:
   - IDLMojo - @goal idl - Generate Java classes and interfaces from AvroIDL 
files (.avdl)
   - IDLProtocolMojo - @goal idl-protocol - The old version of IDLMojo
   
   `IDLMojo` looks like a generic IDL tool so I added the new actions to it. I 
think it would be clearer if it had a more meaningful name likes `IDL2JavaMojo`
   
   And the new Mojos might be: 
   - `IDL2ProtocolMojo` - @goal idl-2-protocol - Generate Json  protocol from 
Avro IDL files (.avdl)
   - `IDL2SchemaMojo` - @goal idl-2-schema - Generate Json Schema from Avro IDL 
files (.avdl)
   Are they OK?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2023-10-30 Thread via GitHub


opwvhk commented on PR #2568:
URL: https://github.com/apache/avro/pull/2568#issuecomment-1784656051

   > Since #1589, we have:
   > 
   > * IDLMojo - @goal idl - Generate Java classes and interfaces from AvroIDL 
files (.avdl)
   > * IDLProtocolMojo - @goal idl-protocol - The old version of IDLMojo
   > 
   > `IDLMojo` looks like a generic IDL tool so I added the new actions to it. 
I think it would be clearer if it had a more meaningful name likes 
`IDL2JavaMojo`
   > 
   > And the new Mojos might be:
   > 
   > * `IDL2ProtocolMojo` - @goal idl-2-protocol - Generate Json protocol from 
Avro IDL files (.avdl)
   > * `IDL2SchemaMojo` - @goal idl-2-schema - Generate Json Schema from Avro 
IDL files (.avdl)
   >   Are they OK?
   
   Looks good, but I'm not a fan of the `-2-` infix; it looks like a separate 
number. I prefer this without the dashes, but `-to-` as infix works as well.
   
   Given that #1589 hasn't been released yet, what do you think of the 
following:
   
   * `IDL2JavaMojo` - @goal idl2java - Generate Java classes and interfaces 
from AvroIDL files (.avdl)
   * `IDLProtocolMojo` - @goal idl-protocol - The old version of IDLMojo
   * `IDL2ProtocolMojo` - @goal idl2protocol - Generate Json protocol from Avro 
IDL files (.avdl)
   * `IDL2SchemaMojo` - @goal idl2schema - Generate Json Schema from Avro IDL 
files (.avdl)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2023-11-01 Thread via GitHub


databius commented on PR #2568:
URL: https://github.com/apache/avro/pull/2568#issuecomment-1790006266

   Thanks for your comment. Just pushed some updates. Please help to take a 
look if you have time @opwvhk 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2023-11-10 Thread via GitHub


databius commented on PR #2568:
URL: https://github.com/apache/avro/pull/2568#issuecomment-1806667009

   Thank @opwvhk  for your approval. May I ask how to merge it to main?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2024-02-07 Thread via GitHub


westsail42 commented on PR #2568:
URL: https://github.com/apache/avro/pull/2568#issuecomment-1933244161

   Any chance of getting this feature into a release soon?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2024-02-08 Thread via GitHub


martin-g commented on code in PR #2568:
URL: https://github.com/apache/avro/pull/2568#discussion_r1482584787


##
doc/content/en/docs/++version++/IDL Language/_index.md:
##
@@ -66,14 +66,18 @@ A Maven plugin is also provided to compile .avdl files. To 
use it, add something
   
 
   
-idl
+idl2java

Review Comment:
   I am not a user of the Java SDK but this looks like a breaking change.
   Old applications that use `idl` will break ?! I see no goal 
`idl` in the list of available 
[goals](https://github.com/apache/avro/pull/2568/files#diff-32270f815b71de14206b8c2ff8a75f363f5b468c12862ddbaa0e8ab415c07361R77-R80).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] AVRO-1524: avro-maven-plugin be able to generate schema from IDL [avro]

2024-02-14 Thread via GitHub


opwvhk commented on code in PR #2568:
URL: https://github.com/apache/avro/pull/2568#discussion_r1489073897


##
doc/content/en/docs/++version++/IDL Language/_index.md:
##
@@ -66,14 +66,18 @@ A Maven plugin is also provided to compile .avdl files. To 
use it, add something
   
 
   
-idl
+idl2java

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org