Ignoring the lines starting with '//' when checking the line length

[jkremser]

When commenting out a bunch of lines/method it's 50:50 that build fails because some line exceeded 120 chars limit.
This has also drawback that people can write comments longer than 120 characters, but I think it's worth changing.

Now, all the projects/modules are relatively small and hence the build time is short, but it gets worse and these types of build failures will get, imho, very annoying.

[pilhuhn]

I think this is a mostly good idea - especially when locally trying changes while debugging.

For longer term we should be diligent enough to remove those commented artifacts before
committing & pushing.

[tsegismont]

You can make your editor show a print margin and you immediately know if you're going to fail the build. And your IDE can also format Javadoc and comments.

[pilhuhn]

Am 06.03.2015 um 10:03 schrieb Thomas Segismont notifications@github.com:

You can make your editor show a print margin and you immediately know if
you're going to fail the build.

The issue I am seeing and am referring to is that I write code, deploy and debug it.
Something is wrong so I comment out a statement.
Now line length + potentially imports are out of whack with checkstyle.
I run mvn install and this fails, so I have to go back to my code, make cosmetic changes,
run maven again just to be able to locally being able to debug my code.
This is what I am referring to.

[tsegismont]

I don't feel the same because my IDE is configured to automatically
format on save (comments included).

If you want to defer to Travis for format and license checking it's easy
to configure your Maven settings file:

   <profiles>
     ...
     <profile>
       <id>defer-checks-to-travis</id>
       <properties>
         <property>license.skip</property>
         <property>checkstyle.skip</property>
       </properties>
     </profile>
     ...
   </profiles>

and then

   <activeProfiles>
     ...
     <activeProfile>defer-checks-to-travis</activeProfile>
     ...
   </activeProfiles>

If those checks are a pain for a majority of developers then I'm fine
with moving them to a profile only active on Travis. However I don't get
the benefit of creating an exception to the rules for comments.

[jkremser]

I haven't found similar feature to the Eclipse's "format on save" in Idea. I guess it'd be possible to workaround this with some macros, but the main issue is that Idea can't do the proper line wrapping during the code formation. It does it correctly when pasting the code to the IDE or when typing in IDE and the line is longer than the limit, but if you use the Ctrl+/ to comment out a line the line wrapping doesn't work. This is the issue.

In ideal world I'd setup an exception for lines starting with '//' to have the line limit to 122, but it's not possible in checkstyle.

Using the -Dcheckstyle.skip with each mvn build and do the proper formatting (without the option) only before pushing the code partially solves the issue, but it's cumbersome.

Just an idea, isn't it possible to hook the checkstyle plugin to the test mvn phase?

[tsegismont]

Le 06/03/2015 13:30, Jirka Kremser a écrit :

I haven't found similar feature to the Eclipse's "format on save" in
Idea. I guess it'd be possible to workaround this with some macros, but
the main issue is that Idea can't do the proper line wrapping during the
code formation. It does it correctly when pasting the code to the IDE or
when typing in IDE and the line is longer than the limit, but if you use
the Ctrl+/ to comment out a line the line wrapping doesn't work. This is
the issue.

In ideal world I'd setup an exception for lines starting with '//' to
have the line limit to 122, but it's not possible in checkstyle.

The day after tomorrow, the new ideal will be 124 :p

Using the |-Dcheckstyle.skip| with each mvn build and do the proper
formatting (without the option) only before pushing the code partially
solves the issue, but it's cumbersome.

I've attached Maven settings snippet in a previous reply which you can
simply copy/paste

Just an idea, isn't it possible to hook the checkstyle plugin to the
test mvn phase?

Good idea. I'd say to the "verify" phase actually. Just like the license
header checks.

[ppalaga]

I tend to be against this as there seem to be enough ways either suppress the problem ( {{-Dcheckstyle.skip}} ) or detect (visible print margin in IDE) and solve (reformat) it.

[jkremser]

The day after tomorrow, the new ideal will be 124 :p

Well,

//
////
////
//

..is not so common pattern.

Ok, so I'll close this PR. Let's hook the checkstyle to the verify mvn phase, this seems to be better solution.

[tsegismont]

PR sent: see hawkular/hawkular-parent-pom#16

[jkremser]

Thanks, Thomas

[pilhuhn]

I think this is a very good solution now in PR 16.
I think "forcing" people to work around those issues with -Dcheckstyle.skip is mostly bad.