search

avro-kotlin / avro4k

Avro format support for Kotlin
Apache License 2.0
194 stars 37 forks source link

Support for different casing (snake, pascal) #94

Closed kossi closed 3 years ago

kossi commented 3 years ago

Hellou,

Added support for snake and pascal casing, there is issue related to this #72 . I'm not 100% sure if this is going to handle every case or how this should be tested.

Probably some people doesn't generate avro schemas from kotlin classes or example my personal use is at the moment a bit different. Anyways this could be quite useful if we can get avro4k to support different casing. To bad kotlin doesn't have implicit imports like scala etc or I was bit confused from where the end-user should actually pass the casing but @thake perhaps you have better knowledge how this should be implemented.

Sorry about the network related examples and please enforce the editorconfig in the build or something. Or I'm it scared to touch some of the files as my editor. And it seems that the current .editorconfig and some personal editor setups are quite different.

sksamuel commented 3 years ago

@thake this looks good to me.

kossi commented 3 years ago

@kossi I'm very sorry for such a late review. I just started a new contract this year and did not find the time. Thanks again for your great pull requests! As part of this PR I would like to introduce an AvroConfiguration class that holds all serialization relevant configuration options. One of these options is the NamingStrategy. Having a wrapper around the currently only option gives us the possibility to add further options without significant refactoring. The new AvroConfiguration should be an optional parameter to the "Avro" class.

What do you think about this?

No problem. Good luck with your new gig.

I think your suggestions sounds good or I implemented AvroConfiguration data class that is a field on the Avro class. And yes it makes those streamBuilders etc much nicer. Hopefully I understood it correctly.

Build are failing because of some dependencies. It is probably ok to bump latest? Is it ok also to upgrade gradle to 6.8?

I wonder should that dokka dependency be dropped for now as those docs are not actually generated. And when eventually somebody have time or knowledge to get them working then bring it back.

thake commented 3 years ago

Thanks for the rework! I just made a small adjustment to the constructor.