9 Replies Latest reply: Mar 28, 2012 11:26 PM by Yong Hao Gao RSS

Formatting...

Clebert Suconic Master

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 Novice

    +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...
    Yong Hao Gao Master

    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 Novice

    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...
    Andy Taylor Master

    if ()

    {

    }

     

    why change?

  • 5. Re: Formatting...
    Jeff Mesnil Master

    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 Master

    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...
    Andy Taylor Master

    - 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 Master

    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...
    Yong Hao Gao Master

    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.