1 2 Previous Next 19 Replies Latest reply on Feb 28, 2011 10:42 AM by kabirkhan

    Checkstyle

    dmlloyd

      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
          dmlloyd

          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
            emuckenhuber

            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
              dmlloyd

              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

                Yeah Ill commit some templates in the repo

                • 5. Re: Checkstyle
                  jason.greene

                  Done see ide-configs/eclipse

                  • 6. Re: Checkstyle
                    jason.greene

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

                    • 7. Re: Checkstyle
                      dimitris

                      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

                        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

                          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

                            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

                              Apparantly it needs autocrlf = true to pass the checkstyle checks

                              • 12. Re: Checkstyle
                                jaikiran

                                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
                                  wolfc

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

                                  • 14. Checkstyle
                                    jpearlin

                                    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