9 Replies Latest reply on Mar 28, 2012 11:26 PM by gaohoward

    Formatting...

    clebert.suconic

      As we talked this morning, we need to reformat our code anyways...

       

       

      We have been using the current format since before I joined JBoss (err.. don't want to say how many years)...

       

      And it seems people are back to the Java style with a few improvements.

       

       

      I think we should move to that, because:

       

      - it already has the codestyle configured.. we can just use it

      - I can easily adapt to new formats, but it's hard to switch back and forth for me.

       

      As we said, that's just a choice we can make...  and I would like to hear from people what they think about doing

       

      if ( ) {

      }

       

      instead of

       

      if ()

      {

      }

       

       

      I don't think it's really a big deal about the format.. we just need to select one before we reformat our code and configure our checkstyle accordingly. (wrong checkstyle would be a compilation failure at that point)

        • 1. Re: Formatting...
          borges

          +1.

           

          In my opinion other motives to change the formatting to match AS7's are:

          • it would be easier for us ('dedicated' HornetQ developers) to work on other JBoss projects;
          • it would be easier for other JBoss developers to collaborate with HornetQ.

           

          On the grounds that using different (idiosyncratic) defaults adds barriers to collaboration.

           

          The way I see it, HornetQ is a (relatively) small project, we are really better off leaving any idiosyncrasies aside and working in sync with methods and choices of the (larger) projects around ours.

           

          It makes it easier for us to collaborate (individually) with other projects, as well as easier for developers from other projects to collaborate with HornetQ. I think that just like this applies to e.g.

          • source revision control (git & GitHub),
          • build tool (Maven),
          • integration server (Jenkins)
          • bug tracker (JIRA)

           

          it also applies to

          • formatting style (say, AS7's rendering of the Java style)
          • 2. Re: Formatting...
            gaohoward

            I would go with

             

            if ()

            {

            }

             

            because that's how I started writing my very first code.

             

            However probably the wise choice is

             

            if() {

            }

             

            as AS7 code base is using it.

             

            Another thing I'd like to suggest is to add a mvn checkstyle task (I believe there is one available) to make sure any newly added code won't break the rules after commit.

            • 3. Re: Formatting...
              borges

              Yong Hao Gao wrote:

               

              Another thing I'd like to suggest is to add a mvn checkstyle task (I believe there is one available) to make sure any newly added code won't break the rules after commit.

              We already have one running on Jenkins, but it is not configured to match the current formatting of the code.

              • 4. Re: Formatting...
                ataylor

                if ()

                {

                }

                 

                why change?

                • 5. Re: Formatting...
                  jmesnil

                  Francisco Borges wrote:

                   

                  In my opinion other motives to change the formatting to match AS7's are:

                  • it would be easier for us ('dedicated' HornetQ developers) to work on other JBoss projects;
                  • it would be easier for other JBoss developers to collaborate with HornetQ.

                   

                  +1. It'd be simpler for me to be able to contribute to HornetQ and AS7 using the same code convention

                   

                  It's configurable in eclipe on a project-basis but I'd prefer to have consistent code formatting across JBoss projects.

                  • 6. Re: Formatting...
                    clebert.suconic

                    Andy Taylor wrote:

                     

                    if ()

                    {

                    }

                     

                    why change?

                     

                    - Consistency with AS7...

                    - We need to reformat the code anyways (there are a few issues done after the recent merges)

                    - We need to add checkstyle. And since the AS7 one seems stable already we would go with that one.

                    • 7. Re: Formatting...
                      ataylor

                      - Consistency with AS7.

                      Why do we need to be consistent with AS7, i dont see any benefits. None of us write much AS7 code so its not really an argument

                      - We need to reformat the code anyways (there are a few issues done after the recent merges)

                      Why do we need to reformat?

                      - We need to add checkstyle. And since the AS7 one seems stable already we would go with that one.

                      They are easy to write, lets just tighten the format rules we already have and use them.

                      • 8. Re: Formatting...
                        clebert.suconic

                        We need to reformat because:

                         

                        - There are a few inconsistencies caused by the lack of checkstyle we had. and some issues that happened during the latest merges from eap

                        • 9. Re: Formatting...
                          gaohoward

                          Several things that are annoying to me:

                           

                          mixed tabs and spaces in code indentation.

                          blank lines but contain white space chars.

                          complicated long single line of code.

                          complicated inner (or inline, annoymous) classes and methods.