You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2020/06/01 10:09:05 UTC

[GitHub] [fineract] vorburger edited a comment on pull request #943: FINERACT-1006 Added spotless to auto format source code

vorburger edited a comment on pull request #943:
URL: https://github.com/apache/fineract/pull/943#issuecomment-636757443


   @thesmallstar So this idea look very interesting, to me; thanks for having investigated this! But I'd like to fully understand exactly what we are doing here a little more myself, before merging it... please know that, even though I've originally suggested you look into Spotless in FINERACT-1006, I've actually not read much about it myself... :smile: so I'm looking for your guidance to fully learn about it myself.
   
   In order to make a full review and merging more easy and avoid conflicts, could I make a suggestion, just from a "practical" point of view? How about in this PR, you revert the .java code changes and ONLY propose whatever we need to add to Gradle (and README, hopefully?) and config files, but do not actually include the result of having run it to change the Java files? That makes it easier for me to completely review it properly, and hopefully eventually merge. We could then, in a separate next step, actually run and merge the result (or I could just do that myself, and merge it "quickly", to avoid delays).
   
   One thing I haven't completely understood yet here is if this "just" adds a new way to Auto Format by launching a certain Gradle task (include doc in README...) or if this, like Checkstyle and Error Prone & Co. will actually verify formatting, and fail the build? Or is the idea that we leave that up to Checkstyle only, but that folks can then use this to make changes comply? Or both? It's something worth clearly documenting in the README, IMHO.
   
   I guess what would be "ideal" is if this would both verify formatting, fail the build if NOK (like Checkstyle; I wouldn't mind if this PR fails to build, because of that), but if such failures would show a clear message à la "Hey buddy, you can easily fix your code's ugly formatting by running `./gradlew something`.... 


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