activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Shannon <christopher.l.shan...@gmail.com>
Subject Re: [DISCUSS] Artemis coding style part 2
Date Thu, 29 Sep 2016 19:32:57 GMT
+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 <clebert.suconic@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 <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/
> >
>
>
>
> --
> Clebert Suconic
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message