Import control file does not handle packages

[Sergey Drozd]

Checkstyle warns lines like this:

package com.github.sevntu.checkstyle.checks.annotation - "Import control file does not handle this package"

for example here, here and here.

please add packages to import control file

[Даниил Ярославцев]

I think, it is time to review and update Checkstyle-For-Checkstyle configuration. Admins, lets vote.

I am OK to reconsider things below:

1. Maximum LineLength - set 100 or even 120 symbols at 'max' property.
2. ImportControlCheck - review 'allowed' and 'forbidden' packages list.
3. Update Checkstyle-For-Checkstyle configuration to not force usage of prefixes like 'm', 'a' at field names, method parameter names, etc.
4. Update Checkstyle-For-Checkstyle configuration to not force creation of 'package-info' file.

[Roman Ivanov]

Hi Daniil,

all of these items are postponed till we finish merged of GSoC.

===========================

1. linelength will be 100 , as it help me in code review from small devices and printed code on paper

2. please provide a reasons

3. I do like that either, but it helps to show that you can enforce custom style on naming, eat your own food. Additionally that helps me to do code review out of Eclipse (95% of my code reviews).

4. It was created on a purpose, I do not remember it ..... might be javadoc generation require it.

thanks,

Roman Ivanov

​

[Даниил Ярославцев]

> all of these items are postponed till we finish merged of GSoC.

ofc., no code format / refactoring issues until GSOC finish (so I didn't create an issue for that and just started the discussion here).

1. As I know, 80 columns is a 'standard' max line length because IBM punch cards in 19th century had this limit (http://en.wikipedia.org/wiki/Punched_card):

In 20th century (~1970) IBM introduced new format of 96-columns punch card (http://fileformats.archiveteam.org/wiki/IBM_96-column_card), which became new standard for "maximum line length" for some time..

Now is 21st century, punch cards are gone and almost any popular data-displaying device could easily display 110 and even 120 symbols per line.

For example, my Android device (5'') seems to display 120 symbols-per-line code well in album mode. In portrait mode it is hard to read code even on my Nexus 7'' (even if code formatted to 80 symbols per line - it is similar to programming on PC with portrait-mode display)

But.. After I tried to print 120-symbols code directly from Eclipse, I noticed that 110 - symbols lines gets wrapped when printing code from Eclipse using default font and margins. There is no way to reduce margins, the only thing is to reduce editor's font size, print and then revert font size back (but this is an ugly trick).

So OK, lets use 100 symbols as max like length. In any case, 100 symbols is much better that 19th century format of 80 symbols  ;)

2. Please, look at Sergey message and take a look at his examples first, then I will provide mine, if they will be still needed.

3. Lets me share some thoughts about this:

> you can enforce custom style on naming, eat your own food

Checkstyle Checks could be easily configured in a bad manner which will lead to ugly code ..

But I don't think that it is the reason to configure Checkstyle in a bad (or very-very and very unpopular) manner and just use it for development to 'eat own food' ;)

Let me explain you the reason why I am OK to remove that prefixes:

a. I did not found any occurence of word 'prefix' in Oracle code convention (http://www.oracle.com/technetwork/java/codeconventions-150003.pdf)

b. Google code convention (https://google-styleguide.googlecode.com/svn/trunk/javaguide.html) says exactly: "In Google Style special prefixes or suffixes, like those seen in the examples name_, mName, s_name and kName, are not used."

c. I did not find any actual opensource Java project which uses prefixes notation (ofc., except for Checkstyle with it's unique code convention).

> Additionally that helps me to do code review out of Eclipse

Code printed directly from IDE keeps coloring and so there is no need to keep prefixes on fields as they will be marked with blue color.

For me that prefixes does not help to review code on GitHub or Android as I don't remember what prefix stands for fields and what for method parameters ) I always forget about 'm' stands for 'member' and that there is no 'f' prefix for fields, and also about method parameter prefix is 'a' instead of 'm' which would be more convenient for me =)

Convenience is very subjective term, so I would agree to ignore Oracle and Google code conventions about prefixes for convenience of somebody in team, but only if anybody else from project admins / contributors think that these prefixes are convenient.

Lets vote.

4. I am not sure that we need 'package-info' files for javaDoc generation at sevntu-checkstyle currently. Lets investigate farther.

=======================================================================

Let me also remind that currently sevntu-checkstyle project does not match our own 'Checkstyle For Checkstyle' config at all ) Brief example: ForbidThrowAnonymousExceptionsCheck.java - no prefixes at all variable names.

So, maybe, lets include checking of sevntu-checkstyle project with 'Checkstyle For Checkstyle' config into maven build (in order to enforce the same code convention on all Checkstyle sources)? Only after that we will really start to 'eat our own food'.

But before fixing 'Checkstyle For Checkstyle' config warnings at sevntu-checkstyle it would be ofc. good to clear that config from some severe restrictions if most part of team does not consider them right.

I think, it should be an iterative process like this:

1. Enforce automatic checking of sevntu-checkstyle code with  'Checkstyle For Checkstyle' config.

2. Discuss some warnings on real-world examples and reduce restrictions, where needed or fix warnings.

3  Do '3' until all Checkstyle warnings gone.

4. Create the PR containing 1-3 code updates.

[Даниил Ярославцев]

I have updated Maximum LineLength to 100 in our formatter. checkstyle-for-checkstyle config seems to be already updated to 100 symbols (https://github.com/checkstyle/checkstyle/blob/master/checkstyle_checks.xml#L106).

Lets continue discussion:

2. We need to add all sevntu-checkstyle packages to the list of packages is being handled by ImportControlCheck. Otherwise, we have to create the separate checkstyle-for_sevntu-checkstyle config.

3. Lets discus in Skype about 'a', 'm' and another prefixes in code.

4. I didn't found any reason for 'package-info' files in main checkstyle project. In any case, code in sevntu-checkstyle 100% does not need 'package-info' files.

Also I propose next update to Checkstyle-for-Checkstyle config and Eclipse formatter: 

I think, we should not force developer to place opening braces at next line everywhere in code, see screenshot below:

[RomanIvanov]

Hi Daniil,

> 4. I didn't found any reason for 'package-info' files in main checkstyle project.

http://www.intertech.com/Blog/whats-package-info-java-for/

> 2. We need to add all sevntu-checkstyle packages to the list of packages is being handled by ImportControlCheck. 

It will be bad main project to have in his list packages from separate project.

I would rather stay stay alone from main configuration, let have a copy config for a while.

> Lets discus in Skype about 'a', 'm' and another prefixes in code.

That notation helps enormously during code-review without IDE (Eclipse). I do 95% of code review in browser.

But I agree notation is old fashioned, and nobody use that style right now (everyone do coding in IDEs).

I agree on that if we update whole project.

All admins should vote on that enormous update.

> I think, we should not force developer to place opening braces at next line everywhere in code,

agree.

thanks,

Roman Ivanov