You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by Christopher Shannon <ch...@gmail.com> on 2016/09/29 18:41:28 UTC
[DISCUSS] Artemis coding style part 2
Hey Everyone,
Last year we had a discussion on the coding style for Artemis and a change
was made to the opening curly brace. However, I've been in the code quite
a bit the past couple weeks doing testing (I am starting to look at what
needs to be done to help move missing features from 5.x) and I've noticed a
couple of things that still don't match up with the 5.x style.
In general I think think we should try and get the style closer to 5.x
because it will make going back and forth between to two code bases easier.
The main thing I noticed is the while the opening brace was moved the
closing curly brace is still on its own line which doesn't match the style
of 5.x. This makes it a bit annoying when working in one project and then
doing work in a different project as suddenly you have to remember where
the curly brace is supposed to go.
For example:
Current format:
try {
//do something
}
catch (Exception cause) {
}
Proposed format, notice that the catch(Exception) part is on the same line
as the closing brace
try {
//do something
} catch (Exception cause) {
}
Thoughts? My preference would be to adopt entire google style guide but I
think at the least should fix the closing curly brace so it matches up with
5.x.
Re: [DISCUSS] Artemis coding style part 2
Posted by Justin Bertram <jb...@apache.com>.
All things being equal I've got no problem with that change.
Assuming this change is ratified my only concern would be with the timing (e.g. minor vs. major version). Changes like this can make things more difficult to back-port upstream fixes to older branches (unless there's some git magic I'm unaware of).
Justin
----- Original Message -----
From: "Christopher Shannon" <ch...@gmail.com>
To: dev@activemq.apache.org
Sent: Thursday, September 29, 2016 1:41:28 PM
Subject: [DISCUSS] Artemis coding style part 2
Hey Everyone,
Last year we had a discussion on the coding style for Artemis and a change
was made to the opening curly brace. However, I've been in the code quite
a bit the past couple weeks doing testing (I am starting to look at what
needs to be done to help move missing features from 5.x) and I've noticed a
couple of things that still don't match up with the 5.x style.
In general I think think we should try and get the style closer to 5.x
because it will make going back and forth between to two code bases easier.
The main thing I noticed is the while the opening brace was moved the
closing curly brace is still on its own line which doesn't match the style
of 5.x. This makes it a bit annoying when working in one project and then
doing work in a different project as suddenly you have to remember where
the curly brace is supposed to go.
For example:
Current format:
try {
//do something
}
catch (Exception cause) {
}
Proposed format, notice that the catch(Exception) part is on the same line
as the closing brace
try {
//do something
} catch (Exception cause) {
}
Thoughts? My preference would be to adopt entire google style guide but I
think at the least should fix the closing curly brace so it matches up with
5.x.
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
On Thu, Sep 29, 2016 at 3:01 PM, Clebert Suconic
<cl...@gmail.com> wrote:
> I don't know about other, but to me this is trivial change to me and I
> don't mind anyways. All I really mind is to have a checkstyle in
> place, whatever that is :)
>
BTW: I didn't mean it wasn't important considering it. I just said
that if you think it needs to be done, lets do it as it is a trivial
change. As long as the checkstyle is updated.
Re: [DISCUSS] Artemis coding style part 2
Posted by Timothy Bish <ta...@gmail.com>.
On 09/29/2016 04:32 PM, Clebert Suconic wrote:
> Anyone working on it already?
>
>
> If not I will do it. (just avoiding duplicating effort)
I'd say go for it, you know the build structure on Artemis better than
the rest of us so probably more likely to spot any trouble
>
> On Thu, Sep 29, 2016 at 3:37 PM, Timothy Bish <ta...@gmail.com> wrote:
>> On 09/29/2016 03:27 PM, Clebert Suconic wrote:
>>> I tried using that checkstye directly and didn't work right away. it
>>> probably needs update versions or something else.
>>>
>>>
>>> +1 to just use the google checks... just needs to be worked out.
>>>
>>>
>>> although I would add:
>>>
>>> <!-- Checks for imports -->
>>> <module name="AvoidStarImport"/>
>>> <module name="RedundantImport"/>
>>> <module name="UnusedImports"/>
>>
>> Those look good, they are actually part of the official Google style guide
>> requirements if you read the doc
>>
>>
>>>
>>> And the sevntu checkstyle I contributed to sevntu:
>>>
>>> <!-- Sevntu checks, http://sevntu-checkstyle.github.io/sevntu.checkstyle/
>>> -->
>>> <module name="DiamondOperatorForVariableDefinition"/>
>>> <module name="RequiredParameterForAnnotation">
>>> <property name="annotationName" value="Parameterized.Parameters"/>
>>> <property name="requiredParameters" value="name"/>
>>> </module>
>>>
>>>
>>>
>>> On Thu, Sep 29, 2016 at 3:13 PM, Timothy Bish <ta...@gmail.com> wrote:
>>>> On 09/29/2016 03:01 PM, Clebert Suconic wrote:
>>>>> I don't know about other, but to me this is trivial change to me and I
>>>>> don't mind anyways. All I really mind is to have a checkstyle in
>>>>> place, whatever that is :)
>>>>>
>>>>> What would need to be changed on the checkstyle rules? did you check?
>>>>
>>>> From the old thread we lazily decided that we should adopt the Google
>>>> style
>>>> guide which is quite close to the 5.x code as it stands now. I didn't
>>>> look
>>>> close enough on the original commit to notice that this was not exactly
>>>> what
>>>> was done but having been in the code over the past few days it has become
>>>> apparent it didn't go as far as I thought it was going to. It is a bit
>>>> irritating trying to bring over code from 5.x as the formatting never
>>>> quite
>>>> matches and breaks the checkstyle rules.
>>>>
>>>> The Google style formatting rules are here:
>>>> https://github.com/google/styleguide all in nice little files that can be
>>>> imported into the IDE of your choice.
>>>>
>>>> For checkstyle configuration to use the Google style that is covered in
>>>> the
>>>> CheckStyle project here:
>>>>
>>>> https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
>>>>
>>>> Previous thread on this is here for reference:
>>>> http://mail-archives.apache.org/mod_mbox/activemq-dev/201508.mbox/browser
>>>>
>>>>
>>>>> On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
>>>>> <ch...@gmail.com> wrote:
>>>>>> Hey Everyone,
>>>>>>
>>>>>> Last year we had a discussion on the coding style for Artemis and a
>>>>>> change
>>>>>> was made to the opening curly brace. However, I've been in the code
>>>>>> quite
>>>>>> a bit the past couple weeks doing testing (I am starting to look at
>>>>>> what
>>>>>> needs to be done to help move missing features from 5.x) and I've
>>>>>> noticed
>>>>>> a
>>>>>> couple of things that still don't match up with the 5.x style.
>>>>>>
>>>>>> In general I think think we should try and get the style closer to 5.x
>>>>>> because it will make going back and forth between to two code bases
>>>>>> easier.
>>>>>> The main thing I noticed is the while the opening brace was moved
>>>>>> the
>>>>>> closing curly brace is still on its own line which doesn't match the
>>>>>> style
>>>>>> of 5.x. This makes it a bit annoying when working in one project and
>>>>>> then
>>>>>> doing work in a different project as suddenly you have to remember
>>>>>> where
>>>>>> the curly brace is supposed to go.
>>>>>>
>>>>>> For example:
>>>>>>
>>>>>> Current format:
>>>>>> try {
>>>>>> //do something
>>>>>> }
>>>>>> catch (Exception cause) {
>>>>>>
>>>>>> }
>>>>>>
>>>>>> Proposed format, notice that the catch(Exception) part is on the same
>>>>>> line
>>>>>> as the closing brace
>>>>>> try {
>>>>>> //do something
>>>>>> } catch (Exception cause) {
>>>>>>
>>>>>> }
>>>>>>
>>>>>> Thoughts? My preference would be to adopt entire google style guide but
>>>>>> I
>>>>>> think at the least should fix the closing curly brace so it matches up
>>>>>> with
>>>>>> 5.x.
>>>>>
>>>>>
>>>> --
>>>> Tim Bish
>>>> twitter: @tabish121
>>>> blog: http://timbish.blogspot.com/
>>>>
>>>
>>
>> --
>> Tim Bish
>> twitter: @tabish121
>> blog: http://timbish.blogspot.com/
>>
>
>
--
Tim Bish
twitter: @tabish121
blog: http://timbish.blogspot.com/
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
Anyone working on it already?
If not I will do it. (just avoiding duplicating effort)
On Thu, Sep 29, 2016 at 3:37 PM, Timothy Bish <ta...@gmail.com> wrote:
> On 09/29/2016 03:27 PM, Clebert Suconic wrote:
>>
>> I tried using that checkstye directly and didn't work right away. it
>> probably needs update versions or something else.
>>
>>
>> +1 to just use the google checks... just needs to be worked out.
>>
>>
>> although I would add:
>>
>> <!-- Checks for imports -->
>> <module name="AvoidStarImport"/>
>> <module name="RedundantImport"/>
>> <module name="UnusedImports"/>
>
>
> Those look good, they are actually part of the official Google style guide
> requirements if you read the doc
>
>
>>
>>
>> And the sevntu checkstyle I contributed to sevntu:
>>
>> <!-- Sevntu checks, http://sevntu-checkstyle.github.io/sevntu.checkstyle/
>> -->
>> <module name="DiamondOperatorForVariableDefinition"/>
>> <module name="RequiredParameterForAnnotation">
>> <property name="annotationName" value="Parameterized.Parameters"/>
>> <property name="requiredParameters" value="name"/>
>> </module>
>>
>>
>>
>> On Thu, Sep 29, 2016 at 3:13 PM, Timothy Bish <ta...@gmail.com> wrote:
>>>
>>> On 09/29/2016 03:01 PM, Clebert Suconic wrote:
>>>>
>>>> I don't know about other, but to me this is trivial change to me and I
>>>> don't mind anyways. All I really mind is to have a checkstyle in
>>>> place, whatever that is :)
>>>>
>>>> What would need to be changed on the checkstyle rules? did you check?
>>>
>>>
>>> From the old thread we lazily decided that we should adopt the Google
>>> style
>>> guide which is quite close to the 5.x code as it stands now. I didn't
>>> look
>>> close enough on the original commit to notice that this was not exactly
>>> what
>>> was done but having been in the code over the past few days it has become
>>> apparent it didn't go as far as I thought it was going to. It is a bit
>>> irritating trying to bring over code from 5.x as the formatting never
>>> quite
>>> matches and breaks the checkstyle rules.
>>>
>>> The Google style formatting rules are here:
>>> https://github.com/google/styleguide all in nice little files that can be
>>> imported into the IDE of your choice.
>>>
>>> For checkstyle configuration to use the Google style that is covered in
>>> the
>>> CheckStyle project here:
>>>
>>> https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
>>>
>>> Previous thread on this is here for reference:
>>> http://mail-archives.apache.org/mod_mbox/activemq-dev/201508.mbox/browser
>>>
>>>
>>>> On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
>>>> <ch...@gmail.com> wrote:
>>>>>
>>>>> Hey Everyone,
>>>>>
>>>>> Last year we had a discussion on the coding style for Artemis and a
>>>>> change
>>>>> was made to the opening curly brace. However, I've been in the code
>>>>> quite
>>>>> a bit the past couple weeks doing testing (I am starting to look at
>>>>> what
>>>>> needs to be done to help move missing features from 5.x) and I've
>>>>> noticed
>>>>> a
>>>>> couple of things that still don't match up with the 5.x style.
>>>>>
>>>>> In general I think think we should try and get the style closer to 5.x
>>>>> because it will make going back and forth between to two code bases
>>>>> easier.
>>>>> The main thing I noticed is the while the opening brace was moved
>>>>> the
>>>>> closing curly brace is still on its own line which doesn't match the
>>>>> style
>>>>> of 5.x. This makes it a bit annoying when working in one project and
>>>>> then
>>>>> doing work in a different project as suddenly you have to remember
>>>>> where
>>>>> the curly brace is supposed to go.
>>>>>
>>>>> For example:
>>>>>
>>>>> Current format:
>>>>> try {
>>>>> //do something
>>>>> }
>>>>> catch (Exception cause) {
>>>>>
>>>>> }
>>>>>
>>>>> Proposed format, notice that the catch(Exception) part is on the same
>>>>> line
>>>>> as the closing brace
>>>>> try {
>>>>> //do something
>>>>> } catch (Exception cause) {
>>>>>
>>>>> }
>>>>>
>>>>> Thoughts? My preference would be to adopt entire google style guide but
>>>>> I
>>>>> think at the least should fix the closing curly brace so it matches up
>>>>> with
>>>>> 5.x.
>>>>
>>>>
>>>>
>>>
>>> --
>>> Tim Bish
>>> twitter: @tabish121
>>> blog: http://timbish.blogspot.com/
>>>
>>
>>
>
>
> --
> Tim Bish
> twitter: @tabish121
> blog: http://timbish.blogspot.com/
>
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by Timothy Bish <ta...@gmail.com>.
On 09/29/2016 03:27 PM, Clebert Suconic wrote:
> I tried using that checkstye directly and didn't work right away. it
> probably needs update versions or something else.
>
>
> +1 to just use the google checks... just needs to be worked out.
>
>
> although I would add:
>
> <!-- Checks for imports -->
> <module name="AvoidStarImport"/>
> <module name="RedundantImport"/>
> <module name="UnusedImports"/>
Those look good, they are actually part of the official Google style
guide requirements if you read the doc
>
>
> And the sevntu checkstyle I contributed to sevntu:
>
> <!-- Sevntu checks, http://sevntu-checkstyle.github.io/sevntu.checkstyle/ -->
> <module name="DiamondOperatorForVariableDefinition"/>
> <module name="RequiredParameterForAnnotation">
> <property name="annotationName" value="Parameterized.Parameters"/>
> <property name="requiredParameters" value="name"/>
> </module>
>
>
>
> On Thu, Sep 29, 2016 at 3:13 PM, Timothy Bish <ta...@gmail.com> wrote:
>> On 09/29/2016 03:01 PM, Clebert Suconic wrote:
>>> I don't know about other, but to me this is trivial change to me and I
>>> don't mind anyways. All I really mind is to have a checkstyle in
>>> place, whatever that is :)
>>>
>>> What would need to be changed on the checkstyle rules? did you check?
>>
>> From the old thread we lazily decided that we should adopt the Google style
>> guide which is quite close to the 5.x code as it stands now. I didn't look
>> close enough on the original commit to notice that this was not exactly what
>> was done but having been in the code over the past few days it has become
>> apparent it didn't go as far as I thought it was going to. It is a bit
>> irritating trying to bring over code from 5.x as the formatting never quite
>> matches and breaks the checkstyle rules.
>>
>> The Google style formatting rules are here:
>> https://github.com/google/styleguide all in nice little files that can be
>> imported into the IDE of your choice.
>>
>> For checkstyle configuration to use the Google style that is covered in the
>> CheckStyle project here:
>> https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
>>
>> Previous thread on this is here for reference:
>> http://mail-archives.apache.org/mod_mbox/activemq-dev/201508.mbox/browser
>>
>>
>>> On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
>>> <ch...@gmail.com> wrote:
>>>> Hey Everyone,
>>>>
>>>> Last year we had a discussion on the coding style for Artemis and a
>>>> change
>>>> was made to the opening curly brace. However, I've been in the code
>>>> quite
>>>> a bit the past couple weeks doing testing (I am starting to look at what
>>>> needs to be done to help move missing features from 5.x) and I've noticed
>>>> a
>>>> couple of things that still don't match up with the 5.x style.
>>>>
>>>> In general I think think we should try and get the style closer to 5.x
>>>> because it will make going back and forth between to two code bases
>>>> easier.
>>>> The main thing I noticed is the while the opening brace was moved the
>>>> closing curly brace is still on its own line which doesn't match the
>>>> style
>>>> of 5.x. This makes it a bit annoying when working in one project and then
>>>> doing work in a different project as suddenly you have to remember where
>>>> the curly brace is supposed to go.
>>>>
>>>> For example:
>>>>
>>>> Current format:
>>>> try {
>>>> //do something
>>>> }
>>>> catch (Exception cause) {
>>>>
>>>> }
>>>>
>>>> Proposed format, notice that the catch(Exception) part is on the same
>>>> line
>>>> as the closing brace
>>>> try {
>>>> //do something
>>>> } catch (Exception cause) {
>>>>
>>>> }
>>>>
>>>> Thoughts? My preference would be to adopt entire google style guide but I
>>>> think at the least should fix the closing curly brace so it matches up
>>>> with
>>>> 5.x.
>>>
>>>
>>
>> --
>> Tim Bish
>> twitter: @tabish121
>> blog: http://timbish.blogspot.com/
>>
>
>
--
Tim Bish
twitter: @tabish121
blog: http://timbish.blogspot.com/
Re: [DISCUSS] Artemis coding style part 2
Posted by Christopher Shannon <ch...@gmail.com>.
+1 for taking the google checkstyle as a baseline and then adding minor
tweaks for whatever we think is necessary.
FYI, from the google checkstyle they just do this for the right curly:
<module name="RightCurly"/>
<module name="RightCurly">
<property name="option" value="alone"/>
<property name="tokens" value="CLASS_DEF, METHOD_DEF, CTOR_DEF,
LITERAL_FOR, LITERAL_WHILE, LITERAL_DO, STATIC_INIT, INSTANCE_INIT"/>
</module>
On Thu, Sep 29, 2016 at 3:27 PM, Clebert Suconic <cl...@gmail.com>
wrote:
> I tried using that checkstye directly and didn't work right away. it
> probably needs update versions or something else.
>
>
> +1 to just use the google checks... just needs to be worked out.
>
>
> although I would add:
>
> <!-- Checks for imports -->
> <module name="AvoidStarImport"/>
> <module name="RedundantImport"/>
> <module name="UnusedImports"/>
>
>
> And the sevntu checkstyle I contributed to sevntu:
>
> <!-- Sevntu checks, http://sevntu-checkstyle.github.io/sevntu.checkstyle/
> -->
> <module name="DiamondOperatorForVariableDefinition"/>
> <module name="RequiredParameterForAnnotation">
> <property name="annotationName" value="Parameterized.Parameters"/>
> <property name="requiredParameters" value="name"/>
> </module>
>
>
>
> On Thu, Sep 29, 2016 at 3:13 PM, Timothy Bish <ta...@gmail.com> wrote:
> > On 09/29/2016 03:01 PM, Clebert Suconic wrote:
> >>
> >> I don't know about other, but to me this is trivial change to me and I
> >> don't mind anyways. All I really mind is to have a checkstyle in
> >> place, whatever that is :)
> >>
> >> What would need to be changed on the checkstyle rules? did you check?
> >
> >
> > From the old thread we lazily decided that we should adopt the Google
> style
> > guide which is quite close to the 5.x code as it stands now. I didn't
> look
> > close enough on the original commit to notice that this was not exactly
> what
> > was done but having been in the code over the past few days it has become
> > apparent it didn't go as far as I thought it was going to. It is a bit
> > irritating trying to bring over code from 5.x as the formatting never
> quite
> > matches and breaks the checkstyle rules.
> >
> > The Google style formatting rules are here:
> > https://github.com/google/styleguide all in nice little files that can
> be
> > imported into the IDE of your choice.
> >
> > For checkstyle configuration to use the Google style that is covered in
> the
> > CheckStyle project here:
> > https://github.com/checkstyle/checkstyle/blob/master/src/
> main/resources/google_checks.xml
> >
> > Previous thread on this is here for reference:
> > http://mail-archives.apache.org/mod_mbox/activemq-dev/
> 201508.mbox/browser
> >
> >
> >>
> >> On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
> >> <ch...@gmail.com> wrote:
> >>>
> >>> Hey Everyone,
> >>>
> >>> Last year we had a discussion on the coding style for Artemis and a
> >>> change
> >>> was made to the opening curly brace. However, I've been in the code
> >>> quite
> >>> a bit the past couple weeks doing testing (I am starting to look at
> what
> >>> needs to be done to help move missing features from 5.x) and I've
> noticed
> >>> a
> >>> couple of things that still don't match up with the 5.x style.
> >>>
> >>> In general I think think we should try and get the style closer to 5.x
> >>> because it will make going back and forth between to two code bases
> >>> easier.
> >>> The main thing I noticed is the while the opening brace was moved
> the
> >>> closing curly brace is still on its own line which doesn't match the
> >>> style
> >>> of 5.x. This makes it a bit annoying when working in one project and
> then
> >>> doing work in a different project as suddenly you have to remember
> where
> >>> the curly brace is supposed to go.
> >>>
> >>> For example:
> >>>
> >>> Current format:
> >>> try {
> >>> //do something
> >>> }
> >>> catch (Exception cause) {
> >>>
> >>> }
> >>>
> >>> Proposed format, notice that the catch(Exception) part is on the same
> >>> line
> >>> as the closing brace
> >>> try {
> >>> //do something
> >>> } catch (Exception cause) {
> >>>
> >>> }
> >>>
> >>> Thoughts? My preference would be to adopt entire google style guide
> but I
> >>> think at the least should fix the closing curly brace so it matches up
> >>> with
> >>> 5.x.
> >>
> >>
> >>
> >
> >
> > --
> > Tim Bish
> > twitter: @tabish121
> > blog: http://timbish.blogspot.com/
> >
>
>
>
> --
> Clebert Suconic
>
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
I tried using that checkstye directly and didn't work right away. it
probably needs update versions or something else.
+1 to just use the google checks... just needs to be worked out.
although I would add:
<!-- Checks for imports -->
<module name="AvoidStarImport"/>
<module name="RedundantImport"/>
<module name="UnusedImports"/>
And the sevntu checkstyle I contributed to sevntu:
<!-- Sevntu checks, http://sevntu-checkstyle.github.io/sevntu.checkstyle/ -->
<module name="DiamondOperatorForVariableDefinition"/>
<module name="RequiredParameterForAnnotation">
<property name="annotationName" value="Parameterized.Parameters"/>
<property name="requiredParameters" value="name"/>
</module>
On Thu, Sep 29, 2016 at 3:13 PM, Timothy Bish <ta...@gmail.com> wrote:
> On 09/29/2016 03:01 PM, Clebert Suconic wrote:
>>
>> I don't know about other, but to me this is trivial change to me and I
>> don't mind anyways. All I really mind is to have a checkstyle in
>> place, whatever that is :)
>>
>> What would need to be changed on the checkstyle rules? did you check?
>
>
> From the old thread we lazily decided that we should adopt the Google style
> guide which is quite close to the 5.x code as it stands now. I didn't look
> close enough on the original commit to notice that this was not exactly what
> was done but having been in the code over the past few days it has become
> apparent it didn't go as far as I thought it was going to. It is a bit
> irritating trying to bring over code from 5.x as the formatting never quite
> matches and breaks the checkstyle rules.
>
> The Google style formatting rules are here:
> https://github.com/google/styleguide all in nice little files that can be
> imported into the IDE of your choice.
>
> For checkstyle configuration to use the Google style that is covered in the
> CheckStyle project here:
> https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
>
> Previous thread on this is here for reference:
> http://mail-archives.apache.org/mod_mbox/activemq-dev/201508.mbox/browser
>
>
>>
>> On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
>> <ch...@gmail.com> wrote:
>>>
>>> Hey Everyone,
>>>
>>> Last year we had a discussion on the coding style for Artemis and a
>>> change
>>> was made to the opening curly brace. However, I've been in the code
>>> quite
>>> a bit the past couple weeks doing testing (I am starting to look at what
>>> needs to be done to help move missing features from 5.x) and I've noticed
>>> a
>>> couple of things that still don't match up with the 5.x style.
>>>
>>> In general I think think we should try and get the style closer to 5.x
>>> because it will make going back and forth between to two code bases
>>> easier.
>>> The main thing I noticed is the while the opening brace was moved the
>>> closing curly brace is still on its own line which doesn't match the
>>> style
>>> of 5.x. This makes it a bit annoying when working in one project and then
>>> doing work in a different project as suddenly you have to remember where
>>> the curly brace is supposed to go.
>>>
>>> For example:
>>>
>>> Current format:
>>> try {
>>> //do something
>>> }
>>> catch (Exception cause) {
>>>
>>> }
>>>
>>> Proposed format, notice that the catch(Exception) part is on the same
>>> line
>>> as the closing brace
>>> try {
>>> //do something
>>> } catch (Exception cause) {
>>>
>>> }
>>>
>>> Thoughts? My preference would be to adopt entire google style guide but I
>>> think at the least should fix the closing curly brace so it matches up
>>> with
>>> 5.x.
>>
>>
>>
>
>
> --
> Tim Bish
> twitter: @tabish121
> blog: http://timbish.blogspot.com/
>
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by Timothy Bish <ta...@gmail.com>.
On 09/29/2016 03:01 PM, Clebert Suconic wrote:
> I don't know about other, but to me this is trivial change to me and I
> don't mind anyways. All I really mind is to have a checkstyle in
> place, whatever that is :)
>
> What would need to be changed on the checkstyle rules? did you check?
From the old thread we lazily decided that we should adopt the Google
style guide which is quite close to the 5.x code as it stands now. I
didn't look close enough on the original commit to notice that this was
not exactly what was done but having been in the code over the past few
days it has become apparent it didn't go as far as I thought it was
going to. It is a bit irritating trying to bring over code from 5.x as
the formatting never quite matches and breaks the checkstyle rules.
The Google style formatting rules are here:
https://github.com/google/styleguide all in nice little files that can
be imported into the IDE of your choice.
For checkstyle configuration to use the Google style that is covered in
the CheckStyle project here:
https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml
Previous thread on this is here for reference:
http://mail-archives.apache.org/mod_mbox/activemq-dev/201508.mbox/browser
>
> On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
> <ch...@gmail.com> wrote:
>> Hey Everyone,
>>
>> Last year we had a discussion on the coding style for Artemis and a change
>> was made to the opening curly brace. However, I've been in the code quite
>> a bit the past couple weeks doing testing (I am starting to look at what
>> needs to be done to help move missing features from 5.x) and I've noticed a
>> couple of things that still don't match up with the 5.x style.
>>
>> In general I think think we should try and get the style closer to 5.x
>> because it will make going back and forth between to two code bases easier.
>> The main thing I noticed is the while the opening brace was moved the
>> closing curly brace is still on its own line which doesn't match the style
>> of 5.x. This makes it a bit annoying when working in one project and then
>> doing work in a different project as suddenly you have to remember where
>> the curly brace is supposed to go.
>>
>> For example:
>>
>> Current format:
>> try {
>> //do something
>> }
>> catch (Exception cause) {
>>
>> }
>>
>> Proposed format, notice that the catch(Exception) part is on the same line
>> as the closing brace
>> try {
>> //do something
>> } catch (Exception cause) {
>>
>> }
>>
>> Thoughts? My preference would be to adopt entire google style guide but I
>> think at the least should fix the closing curly brace so it matches up with
>> 5.x.
>
>
--
Tim Bish
twitter: @tabish121
blog: http://timbish.blogspot.com/
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
I don't know about other, but to me this is trivial change to me and I
don't mind anyways. All I really mind is to have a checkstyle in
place, whatever that is :)
What would need to be changed on the checkstyle rules? did you check?
On Thu, Sep 29, 2016 at 2:41 PM, Christopher Shannon
<ch...@gmail.com> wrote:
> Hey Everyone,
>
> Last year we had a discussion on the coding style for Artemis and a change
> was made to the opening curly brace. However, I've been in the code quite
> a bit the past couple weeks doing testing (I am starting to look at what
> needs to be done to help move missing features from 5.x) and I've noticed a
> couple of things that still don't match up with the 5.x style.
>
> In general I think think we should try and get the style closer to 5.x
> because it will make going back and forth between to two code bases easier.
> The main thing I noticed is the while the opening brace was moved the
> closing curly brace is still on its own line which doesn't match the style
> of 5.x. This makes it a bit annoying when working in one project and then
> doing work in a different project as suddenly you have to remember where
> the curly brace is supposed to go.
>
> For example:
>
> Current format:
> try {
> //do something
> }
> catch (Exception cause) {
>
> }
>
> Proposed format, notice that the catch(Exception) part is on the same line
> as the closing brace
> try {
> //do something
> } catch (Exception cause) {
>
> }
>
> Thoughts? My preference would be to adopt entire google style guide but I
> think at the least should fix the closing curly brace so it matches up with
> 5.x.
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
as promised : https://github.com/apache/activemq-artemis/pull/815
On Thu, Sep 29, 2016 at 10:51 PM, Clebert Suconic
<cl...@gmail.com> wrote:
> I will try to get the change before noon EST
>
>
> On Thursday, September 29, 2016, John D. Ament <jo...@apache.org>
> wrote:
>>
>> Which timezone? Do you eat lunch at noon or 1 pm?
>>
>> John
>>
>> On Thu, Sep 29, 2016 at 9:39 PM Clebert Suconic
>> <cl...@gmail.com>
>> wrote:
>>
>> > I'm almost done. Will send a PR tomorrow morning us time.
>> >
>> > It would help if you guys could avoid merging or committing on master
>> > until
>> > tomorrow lunch time (us time)
>> >
>> > On Thursday, September 29, 2016, Clebert Suconic <
>> > clebert.suconic@gmail.com>
>> > wrote:
>> >
>> > > Yep.. I will make the change.
>> > >
>> > >
>> > > there is no reason it wasn't done before other than.. .oops ;)
>> > >
>> > > On Thu, Sep 29, 2016 at 5:00 PM, Bennet Schulz <mail@bennet-schulz.de
>> > > <javascript:;>> wrote:
>> > > > I personally prefer the 2nd one, but in my opinion it’s not that
>> > > important. Take whatever you want as long as the the whole product
>> > > stays
>> > as
>> > > good as it already is ;-)
>> > > >
>> > > >
>> > > >> Am 29.09.2016 um 20:41 schrieb Christopher Shannon <
>> > > christopher.l.shannon@gmail.com <javascript:;>>:
>> > > >>
>> > > >> Hey Everyone,
>> > > >>
>> > > >> Last year we had a discussion on the coding style for Artemis and a
>> > > change
>> > > >> was made to the opening curly brace. However, I've been in the
>> > > >> code
>> > > quite
>> > > >> a bit the past couple weeks doing testing (I am starting to look at
>> > what
>> > > >> needs to be done to help move missing features from 5.x) and I've
>> > > noticed a
>> > > >> couple of things that still don't match up with the 5.x style.
>> > > >>
>> > > >> In general I think think we should try and get the style closer to
>> > > >> 5.x
>> > > >> because it will make going back and forth between to two code bases
>> > > easier.
>> > > >> The main thing I noticed is the while the opening brace was moved
>> > > >> the
>> > > >> closing curly brace is still on its own line which doesn't match
>> > > >> the
>> > > style
>> > > >> of 5.x. This makes it a bit annoying when working in one project
>> > > >> and
>> > > then
>> > > >> doing work in a different project as suddenly you have to remember
>> > where
>> > > >> the curly brace is supposed to go.
>> > > >>
>> > > >> For example:
>> > > >>
>> > > >> Current format:
>> > > >> try {
>> > > >> //do something
>> > > >> }
>> > > >> catch (Exception cause) {
>> > > >>
>> > > >> }
>> > > >>
>> > > >> Proposed format, notice that the catch(Exception) part is on the
>> > > >> same
>> > > line
>> > > >> as the closing brace
>> > > >> try {
>> > > >> //do something
>> > > >> } catch (Exception cause) {
>> > > >>
>> > > >> }
>> > > >>
>> > > >> Thoughts? My preference would be to adopt entire google style guide
>> > but
>> > > I
>> > > >> think at the least should fix the closing curly brace so it matches
>> > > >> up
>> > > with
>> > > >> 5.x.
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Clebert Suconic
>> > >
>> >
>> >
>> > --
>> > Clebert Suconic
>> >
>
>
>
> --
> Clebert Suconic
>
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
I will try to get the change before noon EST
On Thursday, September 29, 2016, John D. Ament <jo...@apache.org>
wrote:
> Which timezone? Do you eat lunch at noon or 1 pm?
>
> John
>
> On Thu, Sep 29, 2016 at 9:39 PM Clebert Suconic <clebert.suconic@gmail.com
> <javascript:;>>
> wrote:
>
> > I'm almost done. Will send a PR tomorrow morning us time.
> >
> > It would help if you guys could avoid merging or committing on master
> until
> > tomorrow lunch time (us time)
> >
> > On Thursday, September 29, 2016, Clebert Suconic <
> > clebert.suconic@gmail.com <javascript:;>>
> > wrote:
> >
> > > Yep.. I will make the change.
> > >
> > >
> > > there is no reason it wasn't done before other than.. .oops ;)
> > >
> > > On Thu, Sep 29, 2016 at 5:00 PM, Bennet Schulz <mail@bennet-schulz.de
> <javascript:;>
> > > <javascript:;>> wrote:
> > > > I personally prefer the 2nd one, but in my opinion it’s not that
> > > important. Take whatever you want as long as the the whole product
> stays
> > as
> > > good as it already is ;-)
> > > >
> > > >
> > > >> Am 29.09.2016 um 20:41 schrieb Christopher Shannon <
> > > christopher.l.shannon@gmail.com <javascript:;> <javascript:;>>:
> > > >>
> > > >> Hey Everyone,
> > > >>
> > > >> Last year we had a discussion on the coding style for Artemis and a
> > > change
> > > >> was made to the opening curly brace. However, I've been in the code
> > > quite
> > > >> a bit the past couple weeks doing testing (I am starting to look at
> > what
> > > >> needs to be done to help move missing features from 5.x) and I've
> > > noticed a
> > > >> couple of things that still don't match up with the 5.x style.
> > > >>
> > > >> In general I think think we should try and get the style closer to
> 5.x
> > > >> because it will make going back and forth between to two code bases
> > > easier.
> > > >> The main thing I noticed is the while the opening brace was moved
> the
> > > >> closing curly brace is still on its own line which doesn't match the
> > > style
> > > >> of 5.x. This makes it a bit annoying when working in one project and
> > > then
> > > >> doing work in a different project as suddenly you have to remember
> > where
> > > >> the curly brace is supposed to go.
> > > >>
> > > >> For example:
> > > >>
> > > >> Current format:
> > > >> try {
> > > >> //do something
> > > >> }
> > > >> catch (Exception cause) {
> > > >>
> > > >> }
> > > >>
> > > >> Proposed format, notice that the catch(Exception) part is on the
> same
> > > line
> > > >> as the closing brace
> > > >> try {
> > > >> //do something
> > > >> } catch (Exception cause) {
> > > >>
> > > >> }
> > > >>
> > > >> Thoughts? My preference would be to adopt entire google style guide
> > but
> > > I
> > > >> think at the least should fix the closing curly brace so it matches
> up
> > > with
> > > >> 5.x.
> > > >
> > >
> > >
> > >
> > > --
> > > Clebert Suconic
> > >
> >
> >
> > --
> > Clebert Suconic
> >
>
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by "John D. Ament" <jo...@apache.org>.
Which timezone? Do you eat lunch at noon or 1 pm?
John
On Thu, Sep 29, 2016 at 9:39 PM Clebert Suconic <cl...@gmail.com>
wrote:
> I'm almost done. Will send a PR tomorrow morning us time.
>
> It would help if you guys could avoid merging or committing on master until
> tomorrow lunch time (us time)
>
> On Thursday, September 29, 2016, Clebert Suconic <
> clebert.suconic@gmail.com>
> wrote:
>
> > Yep.. I will make the change.
> >
> >
> > there is no reason it wasn't done before other than.. .oops ;)
> >
> > On Thu, Sep 29, 2016 at 5:00 PM, Bennet Schulz <mail@bennet-schulz.de
> > <javascript:;>> wrote:
> > > I personally prefer the 2nd one, but in my opinion it’s not that
> > important. Take whatever you want as long as the the whole product stays
> as
> > good as it already is ;-)
> > >
> > >
> > >> Am 29.09.2016 um 20:41 schrieb Christopher Shannon <
> > christopher.l.shannon@gmail.com <javascript:;>>:
> > >>
> > >> Hey Everyone,
> > >>
> > >> Last year we had a discussion on the coding style for Artemis and a
> > change
> > >> was made to the opening curly brace. However, I've been in the code
> > quite
> > >> a bit the past couple weeks doing testing (I am starting to look at
> what
> > >> needs to be done to help move missing features from 5.x) and I've
> > noticed a
> > >> couple of things that still don't match up with the 5.x style.
> > >>
> > >> In general I think think we should try and get the style closer to 5.x
> > >> because it will make going back and forth between to two code bases
> > easier.
> > >> The main thing I noticed is the while the opening brace was moved the
> > >> closing curly brace is still on its own line which doesn't match the
> > style
> > >> of 5.x. This makes it a bit annoying when working in one project and
> > then
> > >> doing work in a different project as suddenly you have to remember
> where
> > >> the curly brace is supposed to go.
> > >>
> > >> For example:
> > >>
> > >> Current format:
> > >> try {
> > >> //do something
> > >> }
> > >> catch (Exception cause) {
> > >>
> > >> }
> > >>
> > >> Proposed format, notice that the catch(Exception) part is on the same
> > line
> > >> as the closing brace
> > >> try {
> > >> //do something
> > >> } catch (Exception cause) {
> > >>
> > >> }
> > >>
> > >> Thoughts? My preference would be to adopt entire google style guide
> but
> > I
> > >> think at the least should fix the closing curly brace so it matches up
> > with
> > >> 5.x.
> > >
> >
> >
> >
> > --
> > Clebert Suconic
> >
>
>
> --
> Clebert Suconic
>
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
I'm almost done. Will send a PR tomorrow morning us time.
It would help if you guys could avoid merging or committing on master until
tomorrow lunch time (us time)
On Thursday, September 29, 2016, Clebert Suconic <cl...@gmail.com>
wrote:
> Yep.. I will make the change.
>
>
> there is no reason it wasn't done before other than.. .oops ;)
>
> On Thu, Sep 29, 2016 at 5:00 PM, Bennet Schulz <mail@bennet-schulz.de
> <javascript:;>> wrote:
> > I personally prefer the 2nd one, but in my opinion it’s not that
> important. Take whatever you want as long as the the whole product stays as
> good as it already is ;-)
> >
> >
> >> Am 29.09.2016 um 20:41 schrieb Christopher Shannon <
> christopher.l.shannon@gmail.com <javascript:;>>:
> >>
> >> Hey Everyone,
> >>
> >> Last year we had a discussion on the coding style for Artemis and a
> change
> >> was made to the opening curly brace. However, I've been in the code
> quite
> >> a bit the past couple weeks doing testing (I am starting to look at what
> >> needs to be done to help move missing features from 5.x) and I've
> noticed a
> >> couple of things that still don't match up with the 5.x style.
> >>
> >> In general I think think we should try and get the style closer to 5.x
> >> because it will make going back and forth between to two code bases
> easier.
> >> The main thing I noticed is the while the opening brace was moved the
> >> closing curly brace is still on its own line which doesn't match the
> style
> >> of 5.x. This makes it a bit annoying when working in one project and
> then
> >> doing work in a different project as suddenly you have to remember where
> >> the curly brace is supposed to go.
> >>
> >> For example:
> >>
> >> Current format:
> >> try {
> >> //do something
> >> }
> >> catch (Exception cause) {
> >>
> >> }
> >>
> >> Proposed format, notice that the catch(Exception) part is on the same
> line
> >> as the closing brace
> >> try {
> >> //do something
> >> } catch (Exception cause) {
> >>
> >> }
> >>
> >> Thoughts? My preference would be to adopt entire google style guide but
> I
> >> think at the least should fix the closing curly brace so it matches up
> with
> >> 5.x.
> >
>
>
>
> --
> Clebert Suconic
>
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by Clebert Suconic <cl...@gmail.com>.
Yep.. I will make the change.
there is no reason it wasn't done before other than.. .oops ;)
On Thu, Sep 29, 2016 at 5:00 PM, Bennet Schulz <ma...@bennet-schulz.de> wrote:
> I personally prefer the 2nd one, but in my opinion it’s not that important. Take whatever you want as long as the the whole product stays as good as it already is ;-)
>
>
>> Am 29.09.2016 um 20:41 schrieb Christopher Shannon <ch...@gmail.com>:
>>
>> Hey Everyone,
>>
>> Last year we had a discussion on the coding style for Artemis and a change
>> was made to the opening curly brace. However, I've been in the code quite
>> a bit the past couple weeks doing testing (I am starting to look at what
>> needs to be done to help move missing features from 5.x) and I've noticed a
>> couple of things that still don't match up with the 5.x style.
>>
>> In general I think think we should try and get the style closer to 5.x
>> because it will make going back and forth between to two code bases easier.
>> The main thing I noticed is the while the opening brace was moved the
>> closing curly brace is still on its own line which doesn't match the style
>> of 5.x. This makes it a bit annoying when working in one project and then
>> doing work in a different project as suddenly you have to remember where
>> the curly brace is supposed to go.
>>
>> For example:
>>
>> Current format:
>> try {
>> //do something
>> }
>> catch (Exception cause) {
>>
>> }
>>
>> Proposed format, notice that the catch(Exception) part is on the same line
>> as the closing brace
>> try {
>> //do something
>> } catch (Exception cause) {
>>
>> }
>>
>> Thoughts? My preference would be to adopt entire google style guide but I
>> think at the least should fix the closing curly brace so it matches up with
>> 5.x.
>
--
Clebert Suconic
Re: [DISCUSS] Artemis coding style part 2
Posted by Bennet Schulz <ma...@bennet-schulz.de>.
I personally prefer the 2nd one, but in my opinion it’s not that important. Take whatever you want as long as the the whole product stays as good as it already is ;-)
> Am 29.09.2016 um 20:41 schrieb Christopher Shannon <ch...@gmail.com>:
>
> Hey Everyone,
>
> Last year we had a discussion on the coding style for Artemis and a change
> was made to the opening curly brace. However, I've been in the code quite
> a bit the past couple weeks doing testing (I am starting to look at what
> needs to be done to help move missing features from 5.x) and I've noticed a
> couple of things that still don't match up with the 5.x style.
>
> In general I think think we should try and get the style closer to 5.x
> because it will make going back and forth between to two code bases easier.
> The main thing I noticed is the while the opening brace was moved the
> closing curly brace is still on its own line which doesn't match the style
> of 5.x. This makes it a bit annoying when working in one project and then
> doing work in a different project as suddenly you have to remember where
> the curly brace is supposed to go.
>
> For example:
>
> Current format:
> try {
> //do something
> }
> catch (Exception cause) {
>
> }
>
> Proposed format, notice that the catch(Exception) part is on the same line
> as the closing brace
> try {
> //do something
> } catch (Exception cause) {
>
> }
>
> Thoughts? My preference would be to adopt entire google style guide but I
> think at the least should fix the closing curly brace so it matches up with
> 5.x.