[DISCUSS] Artemis Coding Style

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] Artemis Coding Style

christopher.l.shannon
So in general I'm not too picky with coding styles but I just started
looking at the Artemis project a few days ago and something that stood out
to me right away was the use of opening curly braces on a new line.

Virtually ever Java code base I've seen is written in the style using the
opening brace on the same line. (See
http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
 I think that in general it would be a good idea to match up to a style
that most Java developers are used to working with if we want to get more
of the community involved.

I was wondering if anyone would have an issue with changing the style or
what people's thoughts are about this potential change?
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

dkulp

+1!!!!  (or more)

I honestly thought ActiveMQ had a style guide already, but I didn’t find one while searching through the site or anything like IDE setups in the repo or anything.  I guess I always have the stuff from Camel/CXF configured in Eclipse which pretty much uses the javaguide style and never really gave it much thought.  

Dan



> On Aug 4, 2015, at 7:56 AM, Christopher Shannon <[hidden email]> wrote:
>
> So in general I'm not too picky with coding styles but I just started
> looking at the Artemis project a few days ago and something that stood out
> to me right away was the use of opening curly braces on a new line.
>
> Virtually ever Java code base I've seen is written in the style using the
> opening brace on the same line. (See
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
> I think that in general it would be a good idea to match up to a style
> that most Java developers are used to working with if we want to get more
> of the community involved.
>
> I was wondering if anyone would have an issue with changing the style or
> what people's thoughts are about this potential change?

--
Daniel Kulp
[hidden email] - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

clebertsuconic
In reply to this post by christopher.l.shannon
+0. I have no problem on switching coding styles as long as we stick
to one and bind it with checkstyle.

Although, If that will help more people to adopt the codebase and
would help the community, then I am +1000 on that.

The only favor I ask is if anyone is sending a commit to change
codestyles and reconfigure the checkstyle please do it with a pull
request as that will be used also to validate the PR builds.


Maybe we could agree in detail with the checkstyle.xml before we send
a commit doing it though.

On Tue, Aug 4, 2015 at 7:56 AM, Christopher Shannon
<[hidden email]> wrote:

> So in general I'm not too picky with coding styles but I just started
> looking at the Artemis project a few days ago and something that stood out
> to me right away was the use of opening curly braces on a new line.
>
> Virtually ever Java code base I've seen is written in the style using the
> opening brace on the same line. (See
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>  I think that in general it would be a good idea to match up to a style
> that most Java developers are used to working with if we want to get more
> of the community involved.
>
> I was wondering if anyone would have an issue with changing the style or
> what people's thoughts are about this potential change?



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

dkulp
I’d probably start with the CXF configs:

https://git1-us-west.apache.org/repos/asf?p=cxf-build-utils.git;a=tree;f=buildtools/src/main/resources;h=f0a20e01e53e5915a2a298060688c0cf24bc86bf;hb=HEAD

as a base and modify it (likely relax some rules, CXF is pretty strict).   The CXF configs are updated for the latest checkstyle plugins for both Maven and the Eclipse checkstyle plugin.  I know the Camel configs need some update for the new plugins.  Just haven’t gotten around to doing it.

Dan



> On Aug 4, 2015, at 9:45 AM, Clebert Suconic <[hidden email]> wrote:
>
> +0. I have no problem on switching coding styles as long as we stick
> to one and bind it with checkstyle.
>
> Although, If that will help more people to adopt the codebase and
> would help the community, then I am +1000 on that.
>
> The only favor I ask is if anyone is sending a commit to change
> codestyles and reconfigure the checkstyle please do it with a pull
> request as that will be used also to validate the PR builds.
>
>
> Maybe we could agree in detail with the checkstyle.xml before we send
> a commit doing it though.
>
> On Tue, Aug 4, 2015 at 7:56 AM, Christopher Shannon
> <[hidden email]> wrote:
>> So in general I'm not too picky with coding styles but I just started
>> looking at the Artemis project a few days ago and something that stood out
>> to me right away was the use of opening curly braces on a new line.
>>
>> Virtually ever Java code base I've seen is written in the style using the
>> opening brace on the same line. (See
>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>> I think that in general it would be a good idea to match up to a style
>> that most Java developers are used to working with if we want to get more
>> of the community involved.
>>
>> I was wondering if anyone would have an issue with changing the style or
>> what people's thoughts are about this potential change?
>
>
>
> --
> Clebert Suconic

--
Daniel Kulp
[hidden email] - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

Ganesh Murthy
In reply to this post by clebertsuconic
+1 for open curly braces on the same line. I also agree with Clebert.

----- Original Message -----
From: "Clebert Suconic" <[hidden email]>
To: [hidden email]
Sent: Tuesday, August 4, 2015 9:45:57 AM
Subject: Re: [DISCUSS] Artemis Coding Style

+0. I have no problem on switching coding styles as long as we stick
to one and bind it with checkstyle.

Although, If that will help more people to adopt the codebase and
would help the community, then I am +1000 on that.

The only favor I ask is if anyone is sending a commit to change
codestyles and reconfigure the checkstyle please do it with a pull
request as that will be used also to validate the PR builds.


Maybe we could agree in detail with the checkstyle.xml before we send
a commit doing it though.

On Tue, Aug 4, 2015 at 7:56 AM, Christopher Shannon
<[hidden email]> wrote:

> So in general I'm not too picky with coding styles but I just started
> looking at the Artemis project a few days ago and something that stood out
> to me right away was the use of opening curly braces on a new line.
>
> Virtually ever Java code base I've seen is written in the style using the
> opening brace on the same line. (See
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>  I think that in general it would be a good idea to match up to a style
> that most Java developers are used to working with if we want to get more
> of the community involved.
>
> I was wondering if anyone would have an issue with changing the style or
> what people's thoughts are about this potential change?



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

tabish121@gmail.com
In reply to this post by christopher.l.shannon
On 08/04/2015 07:56 AM, Christopher Shannon wrote:

> So in general I'm not too picky with coding styles but I just started
> looking at the Artemis project a few days ago and something that stood out
> to me right away was the use of opening curly braces on a new line.
>
> Virtually ever Java code base I've seen is written in the style using the
> opening brace on the same line. (See
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>  I think that in general it would be a good idea to match up to a style
> that most Java developers are used to working with if we want to get more
> of the community involved.
>
> I was wondering if anyone would have an issue with changing the style or
> what people's thoughts are about this potential change?
>
+1

I think following the javaguide style would make the code much more
approachable.

--
Tim Bish
Sr Software Engineer | RedHat Inc.
[hidden email] | www.redhat.com
twitter: @tabish121
blog: http://timbish.blogspot.com/

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

clebertsuconic
Is this a just do it thing, or we need a vote?

On Tue, Aug 4, 2015 at 10:07 AM, Timothy Bish <[hidden email]> wrote:

> On 08/04/2015 07:56 AM, Christopher Shannon wrote:
>> So in general I'm not too picky with coding styles but I just started
>> looking at the Artemis project a few days ago and something that stood out
>> to me right away was the use of opening curly braces on a new line.
>>
>> Virtually ever Java code base I've seen is written in the style using the
>> opening brace on the same line. (See
>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>>  I think that in general it would be a good idea to match up to a style
>> that most Java developers are used to working with if we want to get more
>> of the community involved.
>>
>> I was wondering if anyone would have an issue with changing the style or
>> what people's thoughts are about this potential change?
>>
> +1
>
> I think following the javaguide style would make the code much more
> approachable.
>
> --
> Tim Bish
> Sr Software Engineer | RedHat Inc.
> [hidden email] | www.redhat.com
> twitter: @tabish121
> blog: http://timbish.blogspot.com/
>



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

dkulp
Lazy consensus is fine.    Wait till tomorrow and if no one objects, just do it.

Dan


> On Aug 4, 2015, at 11:24 AM, Clebert Suconic <[hidden email]> wrote:
>
> Is this a just do it thing, or we need a vote?
>
> On Tue, Aug 4, 2015 at 10:07 AM, Timothy Bish <[hidden email]> wrote:
>> On 08/04/2015 07:56 AM, Christopher Shannon wrote:
>>> So in general I'm not too picky with coding styles but I just started
>>> looking at the Artemis project a few days ago and something that stood out
>>> to me right away was the use of opening curly braces on a new line.
>>>
>>> Virtually ever Java code base I've seen is written in the style using the
>>> opening brace on the same line. (See
>>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>>> I think that in general it would be a good idea to match up to a style
>>> that most Java developers are used to working with if we want to get more
>>> of the community involved.
>>>
>>> I was wondering if anyone would have an issue with changing the style or
>>> what people's thoughts are about this potential change?
>>>
>> +1
>>
>> I think following the javaguide style would make the code much more
>> approachable.
>>
>> --
>> Tim Bish
>> Sr Software Engineer | RedHat Inc.
>> [hidden email] | www.redhat.com
>> twitter: @tabish121
>> blog: http://timbish.blogspot.com/
>>
>
>
>
> --
> Clebert Suconic

--
Daniel Kulp
[hidden email] - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

clebertsuconic
I think I will start with what we have now by just switching the curls
to the same line.. Looking at our checkstyle they are not as strict as
CXF (the one mentioned here).

We can improve later if needed without any problems.

On Tue, Aug 4, 2015 at 11:28 AM, Daniel Kulp <[hidden email]> wrote:

> Lazy consensus is fine.    Wait till tomorrow and if no one objects, just do it.
>
> Dan
>
>
>> On Aug 4, 2015, at 11:24 AM, Clebert Suconic <[hidden email]> wrote:
>>
>> Is this a just do it thing, or we need a vote?
>>
>> On Tue, Aug 4, 2015 at 10:07 AM, Timothy Bish <[hidden email]> wrote:
>>> On 08/04/2015 07:56 AM, Christopher Shannon wrote:
>>>> So in general I'm not too picky with coding styles but I just started
>>>> looking at the Artemis project a few days ago and something that stood out
>>>> to me right away was the use of opening curly braces on a new line.
>>>>
>>>> Virtually ever Java code base I've seen is written in the style using the
>>>> opening brace on the same line. (See
>>>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>>>> I think that in general it would be a good idea to match up to a style
>>>> that most Java developers are used to working with if we want to get more
>>>> of the community involved.
>>>>
>>>> I was wondering if anyone would have an issue with changing the style or
>>>> what people's thoughts are about this potential change?
>>>>
>>> +1
>>>
>>> I think following the javaguide style would make the code much more
>>> approachable.
>>>
>>> --
>>> Tim Bish
>>> Sr Software Engineer | RedHat Inc.
>>> [hidden email] | www.redhat.com
>>> twitter: @tabish121
>>> blog: http://timbish.blogspot.com/
>>>
>>
>>
>>
>> --
>> Clebert Suconic
>
> --
> Daniel Kulp
> [hidden email] - http://dankulp.com/blog
> Talend Community Coder - http://coders.talend.com
>



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

dejanb
+1 for the code style change. I find the code with curly braces on the new
line more verbose than necessary and harder to read. Also, most IDEs
defaults (IntelliJ for example) are set to in-line code style, so it’s
easier to start coding.

Regards
--
Dejan Bosanac
about.me/dejanb

On Tue, Aug 4, 2015 at 5:35 PM, Clebert Suconic <[hidden email]>
wrote:

> I think I will start with what we have now by just switching the curls
> to the same line.. Looking at our checkstyle they are not as strict as
> CXF (the one mentioned here).
>
> We can improve later if needed without any problems.
>
> On Tue, Aug 4, 2015 at 11:28 AM, Daniel Kulp <[hidden email]> wrote:
> > Lazy consensus is fine.    Wait till tomorrow and if no one objects,
> just do it.
> >
> > Dan
> >
> >
> >> On Aug 4, 2015, at 11:24 AM, Clebert Suconic <[hidden email]>
> wrote:
> >>
> >> Is this a just do it thing, or we need a vote?
> >>
> >> On Tue, Aug 4, 2015 at 10:07 AM, Timothy Bish <[hidden email]>
> wrote:
> >>> On 08/04/2015 07:56 AM, Christopher Shannon wrote:
> >>>> So in general I'm not too picky with coding styles but I just started
> >>>> looking at the Artemis project a few days ago and something that
> stood out
> >>>> to me right away was the use of opening curly braces on a new line.
> >>>>
> >>>> Virtually ever Java code base I've seen is written in the style using
> the
> >>>> opening brace on the same line. (See
> >>>>
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
> >>>> I think that in general it would be a good idea to match up to a style
> >>>> that most Java developers are used to working with if we want to get
> more
> >>>> of the community involved.
> >>>>
> >>>> I was wondering if anyone would have an issue with changing the style
> or
> >>>> what people's thoughts are about this potential change?
> >>>>
> >>> +1
> >>>
> >>> I think following the javaguide style would make the code much more
> >>> approachable.
> >>>
> >>> --
> >>> Tim Bish
> >>> Sr Software Engineer | RedHat Inc.
> >>> [hidden email] | www.redhat.com
> >>> twitter: @tabish121
> >>> blog: http://timbish.blogspot.com/
> >>>
> >>
> >>
> >>
> >> --
> >> Clebert Suconic
> >
> > --
> > Daniel Kulp
> > [hidden email] - http://dankulp.com/blog
> > Talend Community Coder - http://coders.talend.com
> >
>
>
>
> --
> Clebert Suconic
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

Robbie Gemmell
In reply to this post by christopher.l.shannon
On 4 August 2015 at 12:56, Christopher Shannon
<[hidden email]> wrote:

> So in general I'm not too picky with coding styles but I just started
> looking at the Artemis project a few days ago and something that stood out
> to me right away was the use of opening curly braces on a new line.
>
> Virtually ever Java code base I've seen is written in the style using the
> opening brace on the same line. (See
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>  I think that in general it would be a good idea to match up to a style
> that most Java developers are used to working with if we want to get more
> of the community involved.
>
> I was wondering if anyone would have an issue with changing the style or
> what people's thoughts are about this potential change?

Making all of the code more consistent seems like a good idea to aid
approachability, so it seems like a good idea to me. I do work on
codebases that use a newline for the braces, but most new stuff I
write has them on the same line. I've tended to prefer them being on
the new line in the past as I think its slightly easier to read
overall, but its defintiely quite verbose at times. I am coming to
like the same line approach the more I use it though, and it does seem
to be the norm.

Robbie
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

clebertsuconic
This is where a pull request fits nicely...


I sent a Pull request with my proposed changes:

https://github.com/apache/activemq-artemis/pull/107


it consists of three commits:

- The checkstyle update:
https://github.com/clebertsuconic/activemq-artemis/commit/f7ef6b08492bb74062d322e73b88d37767fed922

- the changes itself
- the idea settings update (so people can import it to idea).

* I coudln't make exclipe work myself, but I know eclipse will import
the checkstyle.xml to the ide settings.



The best way to evaluate the changes would be through a checkout of
the branch though... the changeet is too big to look at the web
browser but you could get a good idea already of the changes I made.



On Tue, Aug 4, 2015 at 1:03 PM, Robbie Gemmell <[hidden email]> wrote:

> On 4 August 2015 at 12:56, Christopher Shannon
> <[hidden email]> wrote:
>> So in general I'm not too picky with coding styles but I just started
>> looking at the Artemis project a few days ago and something that stood out
>> to me right away was the use of opening curly braces on a new line.
>>
>> Virtually ever Java code base I've seen is written in the style using the
>> opening brace on the same line. (See
>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>>  I think that in general it would be a good idea to match up to a style
>> that most Java developers are used to working with if we want to get more
>> of the community involved.
>>
>> I was wondering if anyone would have an issue with changing the style or
>> what people's thoughts are about this potential change?
>
> Making all of the code more consistent seems like a good idea to aid
> approachability, so it seems like a good idea to me. I do work on
> codebases that use a newline for the braces, but most new stuff I
> write has them on the same line. I've tended to prefer them being on
> the new line in the past as I think its slightly easier to read
> overall, but its defintiely quite verbose at times. I am coming to
> like the same line approach the more I use it though, and it does seem
> to be the norm.
>
> Robbie



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

christopher.l.shannon
I checked out the branch and the curly brace changes look good to me, +1

On Tue, Aug 4, 2015 at 1:20 PM, Clebert Suconic <[hidden email]>
wrote:

> This is where a pull request fits nicely...
>
>
> I sent a Pull request with my proposed changes:
>
> https://github.com/apache/activemq-artemis/pull/107
>
>
> it consists of three commits:
>
> - The checkstyle update:
>
> https://github.com/clebertsuconic/activemq-artemis/commit/f7ef6b08492bb74062d322e73b88d37767fed922
>
> - the changes itself
> - the idea settings update (so people can import it to idea).
>
> * I coudln't make exclipe work myself, but I know eclipse will import
> the checkstyle.xml to the ide settings.
>
>
>
> The best way to evaluate the changes would be through a checkout of
> the branch though... the changeet is too big to look at the web
> browser but you could get a good idea already of the changes I made.
>
>
>
> On Tue, Aug 4, 2015 at 1:03 PM, Robbie Gemmell <[hidden email]>
> wrote:
> > On 4 August 2015 at 12:56, Christopher Shannon
> > <[hidden email]> wrote:
> >> So in general I'm not too picky with coding styles but I just started
> >> looking at the Artemis project a few days ago and something that stood
> out
> >> to me right away was the use of opening curly braces on a new line.
> >>
> >> Virtually ever Java code base I've seen is written in the style using
> the
> >> opening brace on the same line. (See
> >>
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
> >>  I think that in general it would be a good idea to match up to a style
> >> that most Java developers are used to working with if we want to get
> more
> >> of the community involved.
> >>
> >> I was wondering if anyone would have an issue with changing the style or
> >> what people's thoughts are about this potential change?
> >
> > Making all of the code more consistent seems like a good idea to aid
> > approachability, so it seems like a good idea to me. I do work on
> > codebases that use a newline for the braces, but most new stuff I
> > write has them on the same line. I've tended to prefer them being on
> > the new line in the past as I think its slightly easier to read
> > overall, but its defintiely quite verbose at times. I am coming to
> > like the same line approach the more I use it though, and it does seem
> > to be the norm.
> >
> > Robbie
>
>
>
> --
> Clebert Suconic
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

andytaylor
+0, I dont really mind what style is used and happy for it to be
changed. It may make sense tho not to make the changes while anyone is
making big changes, I'm doing some refactoring now and don't fancy a
list of conflicts when i rebase. maybe just before or just after a
release would be a good point?

Andy

On 04/08/15 19:17, Christopher Shannon wrote:

> I checked out the branch and the curly brace changes look good to me, +1
>
> On Tue, Aug 4, 2015 at 1:20 PM, Clebert Suconic <[hidden email]>
> wrote:
>
>> This is where a pull request fits nicely...
>>
>>
>> I sent a Pull request with my proposed changes:
>>
>> https://github.com/apache/activemq-artemis/pull/107
>>
>>
>> it consists of three commits:
>>
>> - The checkstyle update:
>>
>> https://github.com/clebertsuconic/activemq-artemis/commit/f7ef6b08492bb74062d322e73b88d37767fed922
>>
>> - the changes itself
>> - the idea settings update (so people can import it to idea).
>>
>> * I coudln't make exclipe work myself, but I know eclipse will import
>> the checkstyle.xml to the ide settings.
>>
>>
>>
>> The best way to evaluate the changes would be through a checkout of
>> the branch though... the changeet is too big to look at the web
>> browser but you could get a good idea already of the changes I made.
>>
>>
>>
>> On Tue, Aug 4, 2015 at 1:03 PM, Robbie Gemmell <[hidden email]>
>> wrote:
>>> On 4 August 2015 at 12:56, Christopher Shannon
>>> <[hidden email]> wrote:
>>>> So in general I'm not too picky with coding styles but I just started
>>>> looking at the Artemis project a few days ago and something that stood
>> out
>>>> to me right away was the use of opening curly braces on a new line.
>>>>
>>>> Virtually ever Java code base I've seen is written in the style using
>> the
>>>> opening brace on the same line. (See
>>>>
>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>>>>   I think that in general it would be a good idea to match up to a style
>>>> that most Java developers are used to working with if we want to get
>> more
>>>> of the community involved.
>>>>
>>>> I was wondering if anyone would have an issue with changing the style or
>>>> what people's thoughts are about this potential change?
>>>
>>> Making all of the code more consistent seems like a good idea to aid
>>> approachability, so it seems like a good idea to me. I do work on
>>> codebases that use a newline for the braces, but most new stuff I
>>> write has them on the same line. I've tended to prefer them being on
>>> the new line in the past as I think its slightly easier to read
>>> overall, but its defintiely quite verbose at times. I am coming to
>>> like the same line approach the more I use it though, and it does seem
>>> to be the norm.
>>>
>>> Robbie
>>
>>
>>
>> --
>> Clebert Suconic
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

clebertsuconic
What about this:

I do this change next Sunday night / Monday morning. That would give
you or anyone else time to finish current works. If anyone has
anything big pending please send me a note by Friday through this
thread.


I will close that PR for now.

On Tue, Aug 4, 2015 at 3:03 PM, Andy Taylor <[hidden email]> wrote:

> +0, I dont really mind what style is used and happy for it to be changed. It
> may make sense tho not to make the changes while anyone is making big
> changes, I'm doing some refactoring now and don't fancy a list of conflicts
> when i rebase. maybe just before or just after a release would be a good
> point?
>
> Andy
>
>
> On 04/08/15 19:17, Christopher Shannon wrote:
>>
>> I checked out the branch and the curly brace changes look good to me, +1
>>
>> On Tue, Aug 4, 2015 at 1:20 PM, Clebert Suconic
>> <[hidden email]>
>> wrote:
>>
>>> This is where a pull request fits nicely...
>>>
>>>
>>> I sent a Pull request with my proposed changes:
>>>
>>> https://github.com/apache/activemq-artemis/pull/107
>>>
>>>
>>> it consists of three commits:
>>>
>>> - The checkstyle update:
>>>
>>>
>>> https://github.com/clebertsuconic/activemq-artemis/commit/f7ef6b08492bb74062d322e73b88d37767fed922
>>>
>>> - the changes itself
>>> - the idea settings update (so people can import it to idea).
>>>
>>> * I coudln't make exclipe work myself, but I know eclipse will import
>>> the checkstyle.xml to the ide settings.
>>>
>>>
>>>
>>> The best way to evaluate the changes would be through a checkout of
>>> the branch though... the changeet is too big to look at the web
>>> browser but you could get a good idea already of the changes I made.
>>>
>>>
>>>
>>> On Tue, Aug 4, 2015 at 1:03 PM, Robbie Gemmell <[hidden email]>
>>> wrote:
>>>>
>>>> On 4 August 2015 at 12:56, Christopher Shannon
>>>> <[hidden email]> wrote:
>>>>>
>>>>> So in general I'm not too picky with coding styles but I just started
>>>>> looking at the Artemis project a few days ago and something that stood
>>>
>>> out
>>>>>
>>>>> to me right away was the use of opening curly braces on a new line.
>>>>>
>>>>> Virtually ever Java code base I've seen is written in the style using
>>>
>>> the
>>>>>
>>>>> opening brace on the same line. (See
>>>>>
>>>
>>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>>>>>
>>>>>   I think that in general it would be a good idea to match up to a
>>>>> style
>>>>> that most Java developers are used to working with if we want to get
>>>
>>> more
>>>>>
>>>>> of the community involved.
>>>>>
>>>>> I was wondering if anyone would have an issue with changing the style
>>>>> or
>>>>> what people's thoughts are about this potential change?
>>>>
>>>>
>>>> Making all of the code more consistent seems like a good idea to aid
>>>> approachability, so it seems like a good idea to me. I do work on
>>>> codebases that use a newline for the braces, but most new stuff I
>>>> write has them on the same line. I've tended to prefer them being on
>>>> the new line in the past as I think its slightly easier to read
>>>> overall, but its defintiely quite verbose at times. I am coming to
>>>> like the same line approach the more I use it though, and it does seem
>>>> to be the norm.
>>>>
>>>> Robbie
>>>
>>>
>>>
>>>
>>> --
>>> Clebert Suconic
>>>
>>
>



--
Clebert Suconic
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

andytaylor
Works for me

On Tue, 4 Aug 2015 20:22 Clebert Suconic <[hidden email]> wrote:

> What about this:
>
> I do this change next Sunday night / Monday morning. That would give
> you or anyone else time to finish current works. If anyone has
> anything big pending please send me a note by Friday through this
> thread.
>
>
> I will close that PR for now.
>
> On Tue, Aug 4, 2015 at 3:03 PM, Andy Taylor <[hidden email]>
> wrote:
> > +0, I dont really mind what style is used and happy for it to be
> changed. It
> > may make sense tho not to make the changes while anyone is making big
> > changes, I'm doing some refactoring now and don't fancy a list of
> conflicts
> > when i rebase. maybe just before or just after a release would be a good
> > point?
> >
> > Andy
> >
> >
> > On 04/08/15 19:17, Christopher Shannon wrote:
> >>
> >> I checked out the branch and the curly brace changes look good to me, +1
> >>
> >> On Tue, Aug 4, 2015 at 1:20 PM, Clebert Suconic
> >> <[hidden email]>
> >> wrote:
> >>
> >>> This is where a pull request fits nicely...
> >>>
> >>>
> >>> I sent a Pull request with my proposed changes:
> >>>
> >>> https://github.com/apache/activemq-artemis/pull/107
> >>>
> >>>
> >>> it consists of three commits:
> >>>
> >>> - The checkstyle update:
> >>>
> >>>
> >>>
> https://github.com/clebertsuconic/activemq-artemis/commit/f7ef6b08492bb74062d322e73b88d37767fed922
> >>>
> >>> - the changes itself
> >>> - the idea settings update (so people can import it to idea).
> >>>
> >>> * I coudln't make exclipe work myself, but I know eclipse will import
> >>> the checkstyle.xml to the ide settings.
> >>>
> >>>
> >>>
> >>> The best way to evaluate the changes would be through a checkout of
> >>> the branch though... the changeet is too big to look at the web
> >>> browser but you could get a good idea already of the changes I made.
> >>>
> >>>
> >>>
> >>> On Tue, Aug 4, 2015 at 1:03 PM, Robbie Gemmell <
> [hidden email]>
> >>> wrote:
> >>>>
> >>>> On 4 August 2015 at 12:56, Christopher Shannon
> >>>> <[hidden email]> wrote:
> >>>>>
> >>>>> So in general I'm not too picky with coding styles but I just started
> >>>>> looking at the Artemis project a few days ago and something that
> stood
> >>>
> >>> out
> >>>>>
> >>>>> to me right away was the use of opening curly braces on a new line.
> >>>>>
> >>>>> Virtually ever Java code base I've seen is written in the style using
> >>>
> >>> the
> >>>>>
> >>>>> opening brace on the same line. (See
> >>>>>
> >>>
> >>>
> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
> >>>>>
> >>>>>   I think that in general it would be a good idea to match up to a
> >>>>> style
> >>>>> that most Java developers are used to working with if we want to get
> >>>
> >>> more
> >>>>>
> >>>>> of the community involved.
> >>>>>
> >>>>> I was wondering if anyone would have an issue with changing the style
> >>>>> or
> >>>>> what people's thoughts are about this potential change?
> >>>>
> >>>>
> >>>> Making all of the code more consistent seems like a good idea to aid
> >>>> approachability, so it seems like a good idea to me. I do work on
> >>>> codebases that use a newline for the braces, but most new stuff I
> >>>> write has them on the same line. I've tended to prefer them being on
> >>>> the new line in the past as I think its slightly easier to read
> >>>> overall, but its defintiely quite verbose at times. I am coming to
> >>>> like the same line approach the more I use it though, and it does seem
> >>>> to be the norm.
> >>>>
> >>>> Robbie
> >>>
> >>>
> >>>
> >>>
> >>> --
> >>> Clebert Suconic
> >>>
> >>
> >
>
>
>
> --
> Clebert Suconic
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Artemis Coding Style

clebertsuconic
This is now merged, I have re-formated master.

I used a PR so the PR check would validate my changes.
 https://github.com/apache/activemq-artemis/pull/115


Everybody committing to this branch please update your IDE settings. I
have updated IDEA-style.jar under ./etc for the ones using idea.

Eclipse should reuse the checkstyle file but you should still update
your IDE accordingly I believe.



Everyone should do a "mvn -Pdev install" before committing on the
branch, mainly now that the style just changed till you all get used
(this applies to me and anyone used to the former style).

On Tue, Aug 4, 2015 at 3:29 PM, Andy Taylor <[hidden email]> wrote:

> Works for me
>
> On Tue, 4 Aug 2015 20:22 Clebert Suconic <[hidden email]> wrote:
>
>> What about this:
>>
>> I do this change next Sunday night / Monday morning. That would give
>> you or anyone else time to finish current works. If anyone has
>> anything big pending please send me a note by Friday through this
>> thread.
>>
>>
>> I will close that PR for now.
>>
>> On Tue, Aug 4, 2015 at 3:03 PM, Andy Taylor <[hidden email]>
>> wrote:
>> > +0, I dont really mind what style is used and happy for it to be
>> changed. It
>> > may make sense tho not to make the changes while anyone is making big
>> > changes, I'm doing some refactoring now and don't fancy a list of
>> conflicts
>> > when i rebase. maybe just before or just after a release would be a good
>> > point?
>> >
>> > Andy
>> >
>> >
>> > On 04/08/15 19:17, Christopher Shannon wrote:
>> >>
>> >> I checked out the branch and the curly brace changes look good to me, +1
>> >>
>> >> On Tue, Aug 4, 2015 at 1:20 PM, Clebert Suconic
>> >> <[hidden email]>
>> >> wrote:
>> >>
>> >>> This is where a pull request fits nicely...
>> >>>
>> >>>
>> >>> I sent a Pull request with my proposed changes:
>> >>>
>> >>> https://github.com/apache/activemq-artemis/pull/107
>> >>>
>> >>>
>> >>> it consists of three commits:
>> >>>
>> >>> - The checkstyle update:
>> >>>
>> >>>
>> >>>
>> https://github.com/clebertsuconic/activemq-artemis/commit/f7ef6b08492bb74062d322e73b88d37767fed922
>> >>>
>> >>> - the changes itself
>> >>> - the idea settings update (so people can import it to idea).
>> >>>
>> >>> * I coudln't make exclipe work myself, but I know eclipse will import
>> >>> the checkstyle.xml to the ide settings.
>> >>>
>> >>>
>> >>>
>> >>> The best way to evaluate the changes would be through a checkout of
>> >>> the branch though... the changeet is too big to look at the web
>> >>> browser but you could get a good idea already of the changes I made.
>> >>>
>> >>>
>> >>>
>> >>> On Tue, Aug 4, 2015 at 1:03 PM, Robbie Gemmell <
>> [hidden email]>
>> >>> wrote:
>> >>>>
>> >>>> On 4 August 2015 at 12:56, Christopher Shannon
>> >>>> <[hidden email]> wrote:
>> >>>>>
>> >>>>> So in general I'm not too picky with coding styles but I just started
>> >>>>> looking at the Artemis project a few days ago and something that
>> stood
>> >>>
>> >>> out
>> >>>>>
>> >>>>> to me right away was the use of opening curly braces on a new line.
>> >>>>>
>> >>>>> Virtually ever Java code base I've seen is written in the style using
>> >>>
>> >>> the
>> >>>>>
>> >>>>> opening brace on the same line. (See
>> >>>>>
>> >>>
>> >>>
>> http://google.github.io/styleguide/javaguide.html#s4.1.2-blocks-k-r-style)
>> >>>>>
>> >>>>>   I think that in general it would be a good idea to match up to a
>> >>>>> style
>> >>>>> that most Java developers are used to working with if we want to get
>> >>>
>> >>> more
>> >>>>>
>> >>>>> of the community involved.
>> >>>>>
>> >>>>> I was wondering if anyone would have an issue with changing the style
>> >>>>> or
>> >>>>> what people's thoughts are about this potential change?
>> >>>>
>> >>>>
>> >>>> Making all of the code more consistent seems like a good idea to aid
>> >>>> approachability, so it seems like a good idea to me. I do work on
>> >>>> codebases that use a newline for the braces, but most new stuff I
>> >>>> write has them on the same line. I've tended to prefer them being on
>> >>>> the new line in the past as I think its slightly easier to read
>> >>>> overall, but its defintiely quite verbose at times. I am coming to
>> >>>> like the same line approach the more I use it though, and it does seem
>> >>>> to be the norm.
>> >>>>
>> >>>> Robbie
>> >>>
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Clebert Suconic
>> >>>
>> >>
>> >
>>
>>
>>
>> --
>> Clebert Suconic
>>



--
Clebert Suconic