[jira] [Comment Edited] (FINERACT-1006) Auto Format Source Code

["Manthan Surkar (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FINERACT-1006?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17123414#comment-17123414 ] 

Manthan Surkar edited comment on FINERACT-1006 at 6/2/20, 7:08 AM:
-------------------------------------------------------------------

[~vorburger] Yes, this was really a good learning experience. 
On the checkstyle Vs Spotless relation- 
A. Checkstyle is in many ways superset of what spotless has to offer(from what I have observed/read till now) in the way we are using it.
B. We need spotless because, checkstyle although a better option a programmer would want it to fix all its violations itself, and when it comes to code formatting and it won't, it hurts :( . Spotless on the other hand would fix them with a command which is really what we want when it comes to formatting, especially for large code changes.

There are two types of checks:
1. Checkstyle and spotless -one of them accepts. 
2. Checkstyle and spotless -both accept.

For case 2, having them at both places is not a bad idea, also since we follow google checks for spotless it will help us verify and question check styles(secondary benefit). If we then see the slow build or any performance related issues all we would need to do is comment  those particular checks from checkstyle.xml. 

Fot Case 1, I don't  think there would be many of them(only one found as of now). There are no questions asked when we say we need a space before and after =, or we need no more than 120 characters in a line. The only thing that has ambiguity is indentation (Even checkstyle is confused about it :P in some way). For this particularly work intesive job of intending my code spotless is a better option.

In a nutshell (Ignoring indentation checkstyle), Checkstyle has many checks some of which are part of spotless. Checkstyle gives us no ways to automate the formatting, hence we will have the same config as checkstyle set in spotless to automate our formatting. 

 















was (Author: manthan):
[~vorburger] Yes, this was really a good learning experience. 
On the checkstyle Vs Spotless relation- 
A. Checkstyle is in many ways superset of what spotless has to offer(from what I have observed/read till now) in the way we are using it.
B. We need spotless because, checkstyle although a better option a programmer would want it to fix all its violations itself, and when it comes to code formatting and it won't, it hurts :( . Spotless on the other hand would fix them when a command which is really what we want when it comes to formatting, especially for large code changes.

There are two types of checks:
1. Checkstyle and spotless -one of them accepts. 
2. Checkstyle and spotless -both accept.

For case 2, having them at both places is not a bad idea, also since we follow google checks for spotless it will help us verify and question check styles(secondary benefit). If we then see the slow build or any performance related issues all we would need to do is comment  those particular checks from checkstyle.xml. 

Fot Case 1, I don't  think there would be many of them(only one found as of now). There are no questions asked when we say we need a space before and after =, or we need no more than 120 characters in a line. The only thing that has ambiguity is indentation (Even checkstyle is confused about it :P in some way). For this particularly work intesive job of intending my code spotless is a better option.

In a nutshell (Ignoring indentation checkstyle), Checkstyle has many checks some of which are part of spotless. Checkstyle gives us no ways to automate the formatting, hence we will have the same config as checkstyle set in spotless to automate our formatting. 

 














> Auto Format Source Code
> -----------------------
>
>                 Key: FINERACT-1006
>                 URL: https://issues.apache.org/jira/browse/FINERACT-1006
>             Project: Apache Fineract
>          Issue Type: New Feature
>            Reporter: Michael Vorburger
>            Assignee: Manthan Surkar
>            Priority: Major
>             Fix For: 1.4.0
>
>
> In https://github.com/apache/fineract/pull/933 for FINERACT-821, [~Manthan] exploring using an "Auto Source Code Formatter Tool", and I'm creating this issue to further explore and discuss that...
> Manthan originally used https://github.com/google/google-java-format, which seems nice at first, but when I noticed how it's hard-coded to 2 instead of 4 space indentions, I really struggled... I respect that this can seem "silly" (and I agree that on such matters the most important thing is for a project to just pick 1 code formatting convention and then stick to it), but 2 spaces is a very JS/Go rule, and not used in any (open source) Java community that I know of. I personally feel reasonably strongly against adopting a convention (2 spaces) that makes all Fineract code looks different than e.g. JDK's own or Spring's etc. etc. code. Interestingly, even (Google's!) Android AOSP uses 4 not 2 space indention, see https://source.android.com/setup/contribute/code-style, and thus does not follow https://google.github.io/styleguide/javaguide.html#s4.2-block-indentation ... ;=)
> Long story short: I'm actually *FOR* further exploring auto formatting for this project, but if we really do do this, then how about not as a manual "one off", but through integrating Gradle task that any future contributor can easily use? But I'm *AGAINST* us using google-java-format as-is... :P
> Doing a little bit more research on this topic seems justified. Here are some pointers:
> * https://plugins.gradle.org/search?term=format
> * https://plugins.gradle.org/search?term=formatter
> * https://plugins.gradle.org/search?term=code+style
> * https://medium.com/@alexprut/integrate-google-java-style-guide-in-a-java-project-567abb6d7987
> * https://medium.com/@aruny3/improve-code-formatting-on-every-commit-7fbb0cdfdab6
> I have not looked more closely at all of these (but perhaps [~Manthan] you would be interested to do a quick comparison?), but from a quick glance, doesn't https://github.com/diffplug/spotless look pretty neat? It, apparently, supports several formatter plugins... one of them being google-java-format (which I really don't look, because of the 2 spaces), but what looking into making that use our existing https://github.com/apache/fineract/blob/develop/config/fineractdev-eclipse-preferences.epf, which I created and which should match https://github.com/apache/fineract/blob/develop/fineract-provider/config/checkstyle/checkstyle.xml ... that would be cool, but we would get 100% consistent auto-formatting in an IDE AND have the equivalent on the CLI through Gradle - that's the holy grail! ;-)
> [~awasum] [~ptuomola] your input here obviously very welcome (and anyone else's)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)