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:
us...@infra.apache.org


Reply via email to