14 Replies Latest reply on Jul 22, 2008 11:29 AM by gozilla

    Please don't use IDE auto format on commit

    timfox

      If anyone has auto format on commit turned on in their IDE, please disable it.

      It's annoying to have to go and correct a file since it's been munged on commit. (I have had to do this several times).

        • 1. Re: Please don't use IDE auto format on commit
          timfox

          Here's another example of what I believe is auto format munging:

          QueueImpl

           if (status == null) { throw new IllegalStateException(
           "ClientConsumer.handle() should never return null"); }
          


          I've seen quite a few examples like this throughout the code. Guys - *please* check you've turned off auto formatting!

          • 2. Re: Please don't use IDE auto format on commit
            trustin

            What about automatic code cleanups like:

            * trailing space removal
            * Adding @Override annotation automatically
            * Optimizing imports

            ?

            • 3. Re: Please don't use IDE auto format on commit
              clebert.suconic

              This is my own opinion...

              Invoking auto-format manually is okay as an exception (not every time), as long as you're 100% sure nobody else is using the same file (e.g. initial commit)... or when formatting is really screwed up for some reason.

              But we should never use auto-format automatically on commit.

              "Trustin" wrote:

              * trailing space removal


              This might affect SVN diffs on things you didn't change. It might cause some hassle on updates and merges

              "Trustin" wrote:

              * Adding @Override annotation automatically
              * Optimizing imports


              I guess it's better to call Ctrl-Shift-O manually on Eclipse or Idea for the imports. It may change the order of imports for no reason.

              I don't have an opinion about @Override.

              • 4. Re: Please don't use IDE auto format on commit
                skajotde

                Eclipse 3.4 has nice option to fomat on save only last edited lines. I think auto format on edited code is ok and dont mess diffing with old code. Auto format on whole file may corrupt and reach unreadable svn diff ;)

                • 5. Re: Please don't use IDE auto format on commit
                  trustin

                  I agree that automatic removal of trailing spaces can be a problem for a while. However, once all trailing spaces are removed globally (e.g. a huge yet vague commit of changed empty lines), it shouldn't be a problem IMHO.

                  • 6. Re: Please don't use IDE auto format on commit
                    dmlloyd

                    In addition to Eclipse, IDEA (and many other text editors) can support a mode wherein trailing spaces are only trimmed from modified lines. This way the file is slowly cleaned, without introducing massive sweeping code changes.

                    • 7. Re: Please don't use IDE auto format on commit

                      I find it's ok to make formatting changes but just don't do it in a commit that contains code changes. I would do it in a separate commit that only contains formatting changes with a commit message that reflects this.

                      just my 2cents

                      --Aaron

                      • 8. Re: Please don't use IDE auto format on commit
                        timfox

                         

                        "apwalker" wrote:
                        I find it's ok to make formatting changes but just don't do it in a commit that contains code changes. I would do it in a separate commit that only contains formatting changes with a commit message that reflects this.

                        just my 2cents

                        --Aaron


                        +1.

                        Auto format in the same commit can make it hard to see what the original commit was about, hard to view using a diff, and all our commits should be traceable to the JIRA task which should have the JIRA change and only the JIRA change in it.

                        The other problem with auto formatting is everyone seems to have a different idea what "proper" formatting is. This only gets worse to standardise when everyone is using different IDEs.


                        • 9. Re: Please don't use IDE auto format on commit

                           

                          "timfox" wrote:

                          The other problem with auto formatting is everyone seems to have a different idea what "proper" formatting is. This only gets worse to standardise when everyone is using different IDEs.


                          We use checkstyle to enforce our coding standard and fail the build it doesn't conform. Also our CI system will automatically revert any change that causes a build failure.

                          --Aaron

                          • 10. Re: Please don't use IDE auto format on commit
                            skajotde

                             

                            "trustin" wrote:
                            I agree that automatic removal of trailing spaces can be a problem for a while.


                            Eclipse has option to omit whitespace during comparing files from cvs. I have used this with success. When this option is enabled may be another problems?

                            • 11. Re: Please don't use IDE auto format on commit
                              skajotde

                               

                              "timfox" wrote:

                              The other problem with auto formatting is everyone seems to have a different idea what "proper" formatting is. This only gets worse to standardise when everyone is using different IDEs.


                              I think formatting and "proper" formatting isn't so important when editing new files and check style is overuse (some formatting is required) but committing change to file cant disturbs whole file. Yes, I know I don't work on project ;)

                              • 12. Re: Please don't use IDE auto format on commit
                                clebert.suconic

                                 

                                "skajotde" wrote:
                                Yes, I know I don't work on project ;)


                                Your mistake! By participating on this forum *you are* working on the project :-P Don't be shy and you're welcome to contribute more ;-)

                                • 13. Re: Please don't use IDE auto format on commit
                                  skajotde

                                   

                                  "clebert.suconic@jboss.com" wrote:
                                  "skajotde" wrote:
                                  Yes, I know I don't work on project ;)


                                  Your mistake! By participating on this forum *you are* working on the project :-P Don't be shy and you're welcome to contribute more ;-)


                                  Ha ;) Personally I don't like when somebody stays on the sidelines and give advice on my project ;) Now I'm working on my masters and when finish I will think on some work on jboss and later maybe full time job ;)

                                  • 14. Re: Please don't use IDE auto format on commit
                                    gozilla

                                    On a previous project, we had a pre-commit hook reformatting Java code according to the company standard (it was under CVS, but SVN has hooks too).

                                    Also, the eclipse code formatter is usable from the command line.

                                    Francois