Re: [papyrus-rt-dev] Code formatting and checkstyle rules

[Céline JANSSENS <celine.janssens@xxxxxxxxxxx>]

Hi all,

Here is the state of art of the current Checkstyle on Papyrus RT project.

As shown on the printscreen the most frequent warnings are:

1) In the Javadoc the first sentence should end with a period (meaning ., ? or ! )
    This could seem a detail, but it makes the difference. Your Javacode must look as professionnal as possible to allow further automated documentation and a professionnal aspect.
    But I agree , for the existing files it is a lot of time consuming to modify if. So let's this warning for the next times...

2) File does not end with a new line
   This may changed depending on the "system". I guess if I select "lf" for the new line check it should decrease this warnings. But we could meet this warning on the Linux file.
    Does anyone have an idea of what is the proper ends of file to update the the check accordingly ?

3) Missing Javadoc
    The Javadoc helps contributors to understand the different API properly. A clear Javadoc is necessary to help further contributors to understand your code, otherwise, your code will never last and someone else would prefer re-develop it.

4) Naming Convention:
    Due to several cases, i.e:

•    lower case Constants
•    too long variable name
•    Missing Abstract in front of Abstract classes
•    eINSTANCE
•    ...
  

  
    Of course this check can be improved by adding some filter for the exceptions.

5) Multi Return
    This is quite used in the developers habits to have several return statement in the same method. However a single return statement by method improves the maintainability and the extensibility of the code.
    It also helps a lot for the debug as only one breakpoint is required to examine the returned value.

6) Mandatory Default Constructor
    This point could be discussed. But in my point of view it is clearer to have always a constructor, even if it is the default one.

7) Do not use trailing comment on the same line as a java instruction.
    It is a very good habit to comment your code to make it clear and structured. But it is even clearer if the comment is at a separate line to not reach a too long line.

8) The String "X" appears "Y" times.
    If a string is required it should be extracted as a Constant, especially if this string is present more than once in your file;

9) "X" hides a field
    This means that for example:

    protected int numberOfLine;
    public void setNumberOfLine(final int numberOfLine ){ ...}

    The parameter hides the field because it has the same name in a closer scope. This can be confusing.
    As we are not using prefixed variables, it is required to make the difference.

10) Redundant modifier
    This is when you add a public modifier in an Interface which is unnecessary.


Of course Format errors can be easily fixed with a proper formatter.

If we fix those top 10 warnings, we would fix 85 % of the checkstyle Warning.




What is your opinion ?

Best regards

Céline

Le 16/03/2016 18:55, SCHNEKENBURGER Remi 211865 a écrit :

Hi all,

 

Thanks to everyone for this fruitful discussion! I could see here a lot of proposals and volunteers ;)

 

I would propose the following things:

-        Papyrus-RT should reuse the Papyrus default formatter, as this would reduce work when ‘upgrading’ some code from Papyrus-RT to generic Papyrus. It is also very similar to default formatter AFAIR.

-        Papyrus-RT should reuse a checkstyle configuration close to a generic configuration, as for example the one in Sonar. I would prefer not to get too far from existing configurations, and a simple one to begin with. There is already a basic one within Eclipse. Céline proposes one, is there one on Collaborative modeling, Philip?

-        The save action could format the code and do the basic cleaning (import cleaning, etc.)

 

I like very much the idea of pre-commit hooks, because usually the only respected rules are the one that are enforced ;) but I don’t know here how it does fit well with Eclipse, Egit & command line users, etc. I would prefer to avoid manual actions to be performed by developers to setup the environment : Oomph was typically invented to reduce the amount of time needed to start committing on a project while removing manual tasks.

 

To have a concrete plan:

-        @Céline, can you provide a report on current Papyrus-RT code base with a basic checkstyle configuration or the one you usually use?

-        @Philip, can you propose also a checkstyle configuration file from collaborative modeling?

-        @Céline, can you give an update to all committers on the status of the Sonar analysis job on Papyrus-RT? Is this possible to run this analysis on each gerrit contributions?

-        @All, what actions should be done on Save?

-        @All, do you agree to reuse Papyrus formatter?

-        @Christian, which settings would you propose from the Papyrus project on the project .settings - JDT settings and others?

 

Thanks,

Rémi

 

 

-------------------------------------------------------

 

Rémi SCHNEKENBURGER

+33 (0)1 69 08 48 48

CEA Saclay Nano-INNOV

Institut CARNOT CEA LIST

 

www.eclipse.org/papyrus

 

De : papyrus-rt-dev-bounces@xxxxxxxxxxx [mailto:papyrus-rt-dev-bounces@xxxxxxxxxxx] De la part de Ernesto Posse
Envoyé : mercredi 16 mars 2016 18:14
À : papyrus-rt developer discussions <papyrus-rt-dev@xxxxxxxxxxx>
Objet : Re: [papyrus-rt-dev] Code formatting and checkstyle rules

 

So we should set the "Save Actions" individually and commit those to the repo?

 

 

On Wed, Mar 16, 2016 at 12:49 PM, Christian Damus <give.a.damus@xxxxxxxxx> wrote:

Yes, all of the JDT preferences can be managed per project in the .settings folder.

 

Christian

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

 

--

Ernesto Posse

Zeligsoft.com

 

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

--

	Céline JANSSENS Software Engineer +33 (0)2 44 47 23 23
	Mail : cej@xxxxxxxxxxx
6 rue Léonard De Vinci - BP 0119 - 53001 LAVAL Cedex - FRANCE www.all4tec.net