fineract-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [fineract] vorburger commented on pull request #943: FINERACT-1006 Added spotless to auto format source code
Date Thu, 11 Jun 2020 22:50:09 GMT

vorburger commented on pull request #943:
URL: https://github.com/apache/fineract/pull/943#issuecomment-642969559


   @thesmallstar While exploring actually using this myself a little bit, to verify that I
can actually run `./gradlew spotlessApply` myself, that after it ran `./gradlew spotlessCheck`
is happy, AND that both a full `./gradlew build integrationTest` still worked (which it currently
actually does not, but a rebase to grab #990 did the trick), I was blown away about how fast
Spotless was... just 2/20s to reformat almost 2900 files - holy crap! :airplane: 
   
   One thing that I ran into and which surprised me and that could be a minor productivity
PITA is that, apparently, spotless, out of the box, runs at the end, after Java build? I found
that a little surprising, as I would have expected a source code quality (formatting) tool
to run BEFORE Gradle compiles Java. It would make the "turn-around" while developing slightly
more efficient. If that's easy enough to change, perhaps it would be worth to still do that
as part of this PR? Otherwise forget about it.
   
   I've also just realized that we forgot something (minor) here.. now that you've split the
EPF, the respective paragraph in the README still needs to be updated to say that a developer
must now manually import `config/fineractdev-cleanup.xml` and `config/fineractdev-formatter.xml`
as well, agreed? You absolutely and totally should also explain in this doc update that the
very same formatter XML one can use in the (Eclipse) IDE is also used by Spotless to BOTH
validate the formatting AND to autoformat on the CLI - because that's actually really really
cool, and very much worth pointing out explicitly.
   
   >  Travis is still failing though, some violations.
   
   @xurror that's totally expected, here... :smile_cat: I'll handle this.
   
   The question now becomes when exactly we want to merge this (with a second commit, which
I'll make, which will be the massive reformat). I think this kind of thing is "big enough"
that an announcement, with a bit of time given, is appropriate. I'll post something to the
dev list ASAP.


----------------------------------------------------------------
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.

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



Mime
View raw message