1 2 Previous Next 19 Replies Latest reply: Feb 28, 2011 10:42 AM by Kabir Khan RSS

Checkstyle

David Lloyd Master

I've introduced checkstyle into the AS7 codebase.  Right now it prints violations out on the console, however in the (near) future it will cause the build to fail if a violation is detected.

 

This means a few things.

 

  1. Make sure your patches do not introduce new violations.
  2. People with topic branches beware: at some point (soon) the existing violations in trunk will be fixed, which can cause merge conflicts when a rebase is performed unless you rebase using this command:  git --whitespace=fix rebase upstream/master
  3. To avoid productivity loss in the future, make sure your IDE matches the code style, in particular the following points:
    1. Strip trailing whitespace from modified lines
    2. Do not use tab characters under any circumstances
    3. Remove unused and star imports, or imports of classes from the same package
    4. Do not specify redundant modifiers (like "public" on interface methods)

 

The existing violation corrections will be going in over the next couple of days; once they're all gone, the build will no longer proceed if a violation is introduced.

  • 1. Re: Checkstyle
    David Lloyd Master

    OK it took less time than I thought.  All the violations are corrected and pushed, and checkstyle now is being enforced.

     

    Don't forget to use "git --whitespace=fix rebase upstream/master"!  Otherwise your merge will be much more difficult than it needs to be.

  • 2. Re: Checkstyle
    Emanuel Muckenhuber Master

    Maybe we can attach some code templates to Hacking on AS7? Since e.g. the eclipse defaults did not work out for me as well.

  • 3. Re: Checkstyle
    David Lloyd Master

    I know we do have some eclipse users here, but I'm not one of them.  Can someone with some good settings volunteer this information?

  • 4. Re: Checkstyle
    Jason Greene Master

    Yeah Ill commit some templates in the repo

  • 5. Re: Checkstyle
    Jason Greene Master

    Done see ide-configs/eclipse

  • 6. Re: Checkstyle
    Jason Greene Master

    Actually take a look at README.txt, it contains semi-detailed instructions

  • 7. Re: Checkstyle
    Dimitris Andreadis Master

    David Lloyd wrote:

     

    OK it took less time than I thought.  All the violations are corrected and pushed, and checkstyle now is being enforced.

     

    Don't forget to use "git --whitespace=fix rebase upstream/master"!  Otherwise your merge will be much more difficult than it needs to be.

    My git bash shell on Windoze doesn't seem to support --whitepace=fix.

    $ git --version
    git version 1.7.0.2.msysgit.0

  • 8. Re: Checkstyle
    Brian Stansberry Master

    I had the same problem on my Mac with 1.7.1; I needed to move to 1.7.2.

     

    Seems git doesn't follow the "no new features in point releases" rule.

  • 9. Re: Checkstyle
    Brian Stansberry Master

    BTW, the "git --whitespace=fix rebase upstream/master" instruction should only be relevant if you have outstanding development work on a topic branch that you created before David fixed the whitespace and now you want to rebase the topic branch to the master branch before integrating your development work. If you only have a local repo that you use to follow upstream you shouldn't need to do that, you can just "git pull upstream".

  • 10. Re: Checkstyle
    Dimitris Andreadis Master

    I get many "File does not end with a newline." checkstyle errors on Windoze, which makes me think it's something to do with the line endings maybe.

     

    Any idea what is the correct/preferred git 'core.autocrlf' option?

  • 11. Re: Checkstyle
    Dimitris Andreadis Master

    Apparantly it needs autocrlf = true to pass the checkstyle checks

  • 12. Re: Checkstyle
    jaikiran pai Master

    David Lloyd wrote:

     

    I know we do have some eclipse users here, but I'm not one of them. 

    Is there some ready to use IntelliJ IDEA templates for AS7?

  • 13. Re: Checkstyle
    Carlo de Wolf Master

    What's the option in IntelliJ to make sure comments don't have trailing spaces?

  • 14. Checkstyle
    jpearlin Newbie

    I am seeing an issue between the templates in Eclipse and the checkstyle performed by Maven (and maybe I am configuring something wrong).  The provided templates for Eclipse cause a space to be placed before the carriage return for any Javadoc comments on a class or method:

     

    /**

    * Some description

    * <-- space here

    * @param foo ...

    ...

    */

     

    If you manually remove the space, but leave the suggested save actions on (the auto-formatting), the space gets added back by Eclipse (I could not find how to prevent the space from being added...if you look at the Comments section under the Formatter, there is no check box to prevent that line with the space...maybe the template just needs to be edited?).  The end result is that checkstyle check in Maven fails because that line contains a trailing whitespace and therefore cannot build AS.  I got around this by disabling the Format Source Save Action, which is obviously not what we want to do.  Do I just have out-dated templates imported or should I go ahead and look into modifying the template to make the Eclipse save actions consistent with the checkstyle check?

1 2 Previous Next