activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Timothy Bish <tabish...@gmail.com>
Subject Re: [DISCUSS] Artemis coding style part 2
Date Thu, 29 Sep 2016 20:44:36 GMT
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 <tabish121@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 <tabish121@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
>>>>> <christopher.l.shannon@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/


Mime
View raw message