[CONFIGURATION] Formatting braces

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

[CONFIGURATION] Formatting braces

garydgregory
Hi All:

I only hope that this will not turn into a bike shedding thread...

Commons Configuration is one of the few components we have that uses the
formatting rule (enforced by Checkstyle) where braces must be on separate
lines. In the age of lambdas, this is, IMO, lame (a technical term ;-) for
example:

    public static final ConfigurationConsumer<ConfigurationException>
DEFAULT_INCLUDE_LISTENER = e ->
    {
        throw e;
    };

    public static final ConfigurationConsumer<ConfigurationException>
NOOP_INCLUDE_LISTENER = e ->
    {
        // noop
    };

Instead of:

    public static final ConfigurationConsumer<ConfigurationException>
DEFAULT_INCLUDE_LISTENER = e -> { throw e; };

    public static final ConfigurationConsumer<ConfigurationException>
NOOP_INCLUDE_LISTENER = e -> { /* noop */ };

I propose a reformatting to use the "{ on the same line" which means that
blocks go from:

if (test)
{
   // this
}
else
{
  // that
}

to:

if (test) {
   // this
} else {
  // that
}

and so on.

Gary
Reply | Threaded
Open this post in threaded view
|

Re: [CONFIGURATION] Formatting braces

Gilles Sadowski-2
Hello.

+1

Le mer. 11 sept. 2019 à 15:54, Gary Gregory <[hidden email]> a écrit :
>
> Hi All:
>
> I only hope that this will not turn into a bike shedding thread...

A uniform coding style (for all components); that *seems* obvious (too)...

Gilles

>
> Commons Configuration is one of the few components we have that uses the
> formatting rule (enforced by Checkstyle) where braces must be on separate
> lines. In the age of lambdas, this is, IMO, lame (a technical term ;-) for
> example:
>
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e ->
>     {
>         throw e;
>     };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e ->
>     {
>         // noop
>     };
>
> Instead of:
>
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e -> { /* noop */ };
>
> I propose a reformatting to use the "{ on the same line" which means that
> blocks go from:
>
> if (test)
> {
>    // this
> }
> else
> {
>   // that
> }
>
> to:
>
> if (test) {
>    // this
> } else {
>   // that
> }
>
> and so on.
>
> Gary

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [CONFIGURATION] Formatting braces

garydgregory
On Wed, Sep 11, 2019 at 10:12 AM Gilles Sadowski <[hidden email]>
wrote:

> Hello.
>
> +1
>
> Le mer. 11 sept. 2019 à 15:54, Gary Gregory <[hidden email]> a
> écrit :
> >
> > Hi All:
> >
> > I only hope that this will not turn into a bike shedding thread...
>
> A uniform coding style (for all components); that *seems* obvious (too)...
>

Well, yes, it would be nice to share Checkstyle, SpotBugs, and PMD
configurations but I am only taking one small manageable step here.

Gary


> Gilles
>
> >
> > Commons Configuration is one of the few components we have that uses the
> > formatting rule (enforced by Checkstyle) where braces must be on separate
> > lines. In the age of lambdas, this is, IMO, lame (a technical term ;-)
> for
> > example:
> >
> >     public static final ConfigurationConsumer<ConfigurationException>
> > DEFAULT_INCLUDE_LISTENER = e ->
> >     {
> >         throw e;
> >     };
> >
> >     public static final ConfigurationConsumer<ConfigurationException>
> > NOOP_INCLUDE_LISTENER = e ->
> >     {
> >         // noop
> >     };
> >
> > Instead of:
> >
> >     public static final ConfigurationConsumer<ConfigurationException>
> > DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
> >
> >     public static final ConfigurationConsumer<ConfigurationException>
> > NOOP_INCLUDE_LISTENER = e -> { /* noop */ };
> >
> > I propose a reformatting to use the "{ on the same line" which means that
> > blocks go from:
> >
> > if (test)
> > {
> >    // this
> > }
> > else
> > {
> >   // that
> > }
> >
> > to:
> >
> > if (test) {
> >    // this
> > } else {
> >   // that
> > }
> >
> > and so on.
> >
> > Gary
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [CONFIGURATION] Formatting braces

sebb-2-2
On Wed, 11 Sep 2019 at 15:15, Gary Gregory <[hidden email]> wrote:

>
> On Wed, Sep 11, 2019 at 10:12 AM Gilles Sadowski <[hidden email]>
> wrote:
>
> > Hello.
> >
> > +1
> >
> > Le mer. 11 sept. 2019 à 15:54, Gary Gregory <[hidden email]> a
> > écrit :
> > >
> > > Hi All:
> > >
> > > I only hope that this will not turn into a bike shedding thread...
> >
> > A uniform coding style (for all components); that *seems* obvious (too)...
> >
>
> Well, yes, it would be nice to share Checkstyle, SpotBugs, and PMD
> configurations but I am only taking one small manageable step here.

+1 in this case; separate lines don't make sense for lambdas

> Gary
>
>
> > Gilles
> >
> > >
> > > Commons Configuration is one of the few components we have that uses the
> > > formatting rule (enforced by Checkstyle) where braces must be on separate
> > > lines. In the age of lambdas, this is, IMO, lame (a technical term ;-)
> > for
> > > example:
> > >
> > >     public static final ConfigurationConsumer<ConfigurationException>
> > > DEFAULT_INCLUDE_LISTENER = e ->
> > >     {
> > >         throw e;
> > >     };
> > >
> > >     public static final ConfigurationConsumer<ConfigurationException>
> > > NOOP_INCLUDE_LISTENER = e ->
> > >     {
> > >         // noop
> > >     };
> > >
> > > Instead of:
> > >
> > >     public static final ConfigurationConsumer<ConfigurationException>
> > > DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
> > >
> > >     public static final ConfigurationConsumer<ConfigurationException>
> > > NOOP_INCLUDE_LISTENER = e -> { /* noop */ };
> > >
> > > I propose a reformatting to use the "{ on the same line" which means that
> > > blocks go from:
> > >
> > > if (test)
> > > {
> > >    // this
> > > }
> > > else
> > > {
> > >   // that
> > > }
> > >
> > > to:
> > >
> > > if (test) {
> > >    // this
> > > } else {
> > >   // that
> > > }
> > >
> > > and so on.
> > >
> > > Gary
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [CONFIGURATION] Formatting braces

Thomas Vandahl
In reply to this post by garydgregory
On 11.09.19 15:53, Gary Gregory wrote:
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e -> { /* noop */ };

+1 for these.

> I propose a reformatting to use the "{ on the same line" which means that
> blocks go from:
>
> if (test)
> {
>    // this
> }
> else
> {
>   // that
> }
>
> to:
>
> if (test) {
>    // this
> } else {
>   // that
> }

-1 here. At least not for all components.

Bye, Thomas


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [CONFIGURATION] Formatting braces

garydgregory
On Thu, Sep 12, 2019 at 1:11 PM Thomas Vandahl <[hidden email]> wrote:

> On 11.09.19 15:53, Gary Gregory wrote:
> >     public static final ConfigurationConsumer<ConfigurationException>
> > DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
> >
> >     public static final ConfigurationConsumer<ConfigurationException>
> > NOOP_INCLUDE_LISTENER = e -> { /* noop */ };
>
> +1 for these.
>
> > I propose a reformatting to use the "{ on the same line" which means that
> > blocks go from:
> >
> > if (test)
> > {
> >    // this
> > }
> > else
> > {
> >   // that
> > }
> >
> > to:
> >
> > if (test) {
> >    // this
> > } else {
> >   // that
> > }
>
> -1 here. At least not for all components.
>

I was only talking about Configuration here.

Gary


>
> Bye, Thomas
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [CONFIGURATION] Formatting braces

James Ring
In reply to this post by garydgregory
What about https://github.com/google/google-java-format ? Just require
that it be run on check-in, all these style questions go away (as a
bonus, it looks nice too).

On Wed, Sep 11, 2019 at 6:54 AM Gary Gregory <[hidden email]> wrote:

>
> Hi All:
>
> I only hope that this will not turn into a bike shedding thread...
>
> Commons Configuration is one of the few components we have that uses the
> formatting rule (enforced by Checkstyle) where braces must be on separate
> lines. In the age of lambdas, this is, IMO, lame (a technical term ;-) for
> example:
>
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e ->
>     {
>         throw e;
>     };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e ->
>     {
>         // noop
>     };
>
> Instead of:
>
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e -> { /* noop */ };
>
> I propose a reformatting to use the "{ on the same line" which means that
> blocks go from:
>
> if (test)
> {
>    // this
> }
> else
> {
>   // that
> }
>
> to:
>
> if (test) {
>    // this
> } else {
>   // that
> }
>
> and so on.
>
> Gary

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

RE: [CONFIGURATION] Formatting braces

Mark Roberts
That's exactly what we do with all our projects here at UW/PLSE.
Mark

-----Original Message-----
From: James Ring [mailto:[hidden email]]
Sent: Friday, September 13, 2019 6:44 AM
To: Commons Developers List <[hidden email]>
Subject: Re: [CONFIGURATION] Formatting braces

What about https://github.com/google/google-java-format ? Just require that it be run on check-in, all these style questions go away (as a bonus, it looks nice too).

On Wed, Sep 11, 2019 at 6:54 AM Gary Gregory <[hidden email]> wrote:

>
> Hi All:
>
> I only hope that this will not turn into a bike shedding thread...
>
> Commons Configuration is one of the few components we have that uses
> the formatting rule (enforced by Checkstyle) where braces must be on
> separate lines. In the age of lambdas, this is, IMO, lame (a technical
> term ;-) for
> example:
>
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e ->
>     {
>         throw e;
>     };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e ->
>     {
>         // noop
>     };
>
> Instead of:
>
>     public static final ConfigurationConsumer<ConfigurationException>
> DEFAULT_INCLUDE_LISTENER = e -> { throw e; };
>
>     public static final ConfigurationConsumer<ConfigurationException>
> NOOP_INCLUDE_LISTENER = e -> { /* noop */ };
>
> I propose a reformatting to use the "{ on the same line" which means
> that blocks go from:
>
> if (test)
> {
>    // this
> }
> else
> {
>   // that
> }
>
> to:
>
> if (test) {
>    // this
> } else {
>   // that
> }
>
> and so on.
>
> Gary

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]