Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [che-dev] Che developers formatting rules for Java

I am -1 for automated sources formatting by maven or any other tool.
What is better? 

1. User getUser() { return user; } 

or 

2. User getUser() {
    return user;
}

Even if you think that 2 is better than 1 it's just your personal preference.

It's hard to create & support formatting tool for different IDEs, if formatting rules are not simple
then they depend on IDE ability to deal with complicated structures like AST etc.

So the tool may force developer to use some tricks in source code to allow the tool to format the source in appropriate way
(like sunix did in his PR "//").


Let's say the formatting tool doesn't handle the code you think is readable and ok, e.g:

Builder.create()
           .tag(...)
               .innerTag(...)
               .endTag();
           .endTag()
           .tag(...)
           // ...
           .build()

The example of such code: https://github.com/eclipse/che/blob/1648a3b57fbb0381ce44421bfe6d21f4d740356e/plugins/plugin-docker/che-plugin-openshift-client/src/main/java/org/eclipse/che/plugin/openshift/client/OpenShiftConnector.java#L741-L754

So what would you do, exclude the source file from formatting build phase?
Does it really worth it?
---

On the other hand there are rules like "use camel case for method names starting from non-capital letters" so it's "getUser" but not "GetUser" or "get_user", 
which are basically not about formatting but rather about naming.

There are also rules about creating and structuring source files like this one "java source class must contain no more than 1 top level class".
--- 

Conclusion.

I would create a short list of rules mostly about naming, structuring and indentation and use it as our contribution guidelines instead 
of documentation we have right now, which is close to Genady's point of view.

I think that the next flow is more than enough: 
 - each time someone makes PR he's asked to read "how to contribute" guide -> https://github.com/eclipse/che/wiki/How-To-Contribute.
 - PR reviewer might refer the guidelines(short list) if he thinks that the code he's reviewing breaks it


On Wed, Mar 29, 2017 at 1:10 PM, Sun Tan <sutan@xxxxxxxxxx> wrote:

Problem with carriage return is that after several month someone who uses formatter that doesn't need trailing slashes will remove it to cleanup the code.
I can't say that it is fault of that developer because he removed it. We should not forbid cleaning the code. 
So carriage return should be controlled by code formatter. 

I guess it would be reviewed and some question would come "why are you removing these trailing slashes ?"
 
about #1, the only common tool across all IDE is maven
So I should say that we should have a maven plugin allowing to format the code (the same code convention could be used with Eclipse IDE, IntelliJ, Eclipse Che, another IDE) but at the end we could still use maven to check or format.

In this mailing list, I would say that the only common tool is Eclipse Che ... 
Whatever the tool we use, a maven plugin or other, the formatting rules we choose should be available in Eclipse Che. This is what we should do at least.
 
about #2, I would avoid any "developer's touch". Formatter is the rule. (we can complain on the format but once it is pick-up, it should be applied without exceptions)
because "he choice of the developer to make the code easier to read" is subjective. Some developers will like in one line, some on separate lines while the formatter will always be the same.
A lot of things is subjective when we write code: variable name, method name, order of parameter, etc ...
Also, I think it's impossible to have a perfect formatter ... and at the end it will have to write unit test to make sure that all the change we are making to the formatter doesn't break the formatting rules defined for another piece code. We are not robot.

 

On Wed, Mar 29, 2017 at 10:51 AM, Sun Tan <sutan@xxxxxxxxxx> wrote:
I'd like to come up with some discussion we hard during reviews: https://github.com/eclipse/che/pull/4556/files/ba9e3984575c49f8f39a56db1df2e07b53786b3e#diff-ab312e4ea4cd0eee68fe53472bab93e1 

1. What tool to use to format code in Java ?
2. Usage of // to force Carriage return in some case

# 1. Some are using Intellij, the others Eclipse IDE and some others are using Eclipse Che as IDE.
For me, it makes sense to have a common formatter tool whatever the IDE we are using. It may also work from the command line and should be opensource. It should be possible to format part of the file. It should support Java files at least.
At the moment I use Che to code on Che so if someone has a formatter tool that meets all the previous requirements, I can take it ... otherwise if there isn't such a tool we should all use Che formatter to format our code :)

# 2. Usage of // to force Carriage return in some case
I had some questions about trailling // that I use to force cariage return when the formatter doesn't want to. The purpose is to avoid someone else that run a formatter to loose the carriage return that was there originally. I think this is the choice of the developer to make the code easier to read and to choose wether a line of code should be break into 2 line of code or not. Pretty much like when we add \ at the end of a bash script or Dockerfile.

--
Sun Tan
Senior Software Engineer
Eclipse Che @ Red Hat

_______________________________________________
che-dev mailing list
che-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/che-dev




--
Florent Benoit // Codenvy
_______________________________________________
che-dev mailing list
che-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/che-dev
--
Sun Tan
Senior Software Engineer
Eclipse Che @ Red Hat

_______________________________________________
che-dev mailing list
che-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/che-dev



Back to the top