search

makubi / avrohugger-maven-plugin

Maven plugin for generating Scala case classes and ADTs from Apache Avro schemas, datafiles, and protocols
Apache License 2.0
9 stars 9 forks source link

Fix sorting algorithm #54

Closed tyger closed 3 years ago

tyger commented 3 years ago

Current approach for sorting process folder by folder. So that files are sorted within each folder independently so that if dependent files are in subfolder it going to fail to sort them in the right order. This fix changes logic to first read all required files recursively and then sort them with avrohugger-filesorter.

makubi commented 3 years ago

Please have a look at https://github.com/makubi/avrohugger-maven-plugin/commit/74621a9433a31f4645fbac9dabf1402eace6afb5. What do you think?

tyger commented 3 years ago

Hi @makubi, thanks for reviewing it.

tyger commented 3 years ago

np, I will squash commits. Let me double check the test, it was fine when I was experimenting, but maybe I changed something.

What comes to your proposed commit I think it makes sense only if you expecting alternative implementations of file lister to be provided by developers. Maybe it is not needed at this stage - I don't see right now what that alternative may be. However this flexibility might be useful in the future. Do you want me to add this to my PR?

tyger commented 3 years ago

@makubi I've updated PR with suggested code. Also I've checked the test that I had added originally and it did fail on the old version of sorter, so at least in Windows the behaviour is reproducible.

tyger commented 3 years ago

@makubi decided to keep FileListHelper to not introduce another round of changes. Commits are squashed. Please have a look again.