[DISCUSS] Proposed formatter change: 100 char lines

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

[DISCUSS] Proposed formatter change: 100 char lines

Christopher Tubbs-2
Primarily for accessibility reasons (screen space with a comfortable font),
but also to support readability for devs working on sensibly-sized screens,
I want to change our formatter to format with 100 char line length instead
of its current 160.

Many of our files need to be reformatted anyway, because the current
formatter is configured incorrectly for Java 8 lambda syntax and needs to
be fixed, so this might be a good opportunity to make the switch.

Also, at this point I think it is sensible to require Java 8 to build
Accumulo... even when building older branches. (Accumulo 1.x will still
support running on Java 7, of course, but Java 8 would be required to build
it). The reason for this requirement is that in order to reduce merge
conflicts and merge bugs between branches, I'd like to update the
formatting across all branches, but the formatter which supports this
syntax requires Java 8 to run. The alternative to requiring Java 8 would be
to only run the formatter when building with Java 8... and skip formatting
if building with Java 7, which might result in some unformatted
contributions, depending on the JRE version used to build.
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Proposed formatter change: 100 char lines

J. Mark Owens
I would be in favor of this change. As my eyes continue to get weaker, I
have to use larger fonts resulting in a lot of the code being off the
screen. The constant need to scroll horizontally can get irritating.


------ Original Message ------
From: "Christopher" <[hidden email]>
To: "accumulo-dev" <[hidden email]>
Sent: 2/15/2018 11:24:31 PM
Subject: [DISCUSS] Proposed formatter change: 100 char lines

>Primarily for accessibility reasons (screen space with a comfortable
>font),
>but also to support readability for devs working on sensibly-sized
>screens,
>I want to change our formatter to format with 100 char line length
>instead
>of its current 160.
>
>Many of our files need to be reformatted anyway, because the current
>formatter is configured incorrectly for Java 8 lambda syntax and needs
>to
>be fixed, so this might be a good opportunity to make the switch.
>
>Also, at this point I think it is sensible to require Java 8 to build
>Accumulo... even when building older branches. (Accumulo 1.x will still
>support running on Java 7, of course, but Java 8 would be required to
>build
>it). The reason for this requirement is that in order to reduce merge
>conflicts and merge bugs between branches, I'd like to update the
>formatting across all branches, but the formatter which supports this
>syntax requires Java 8 to run. The alternative to requiring Java 8
>would be
>to only run the formatter when building with Java 8... and skip
>formatting
>if building with Java 7, which might result in some unformatted
>contributions, depending on the JRE version used to build.

Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Proposed formatter change: 100 char lines

Mike Walch-2
+1. I prefer a 100 character column limit. It's also the standard in the
Google Java Style Guide:

https://google.github.io/styleguide/javaguide.html#s4.4-column-limit

On Fri, Feb 16, 2018 at 8:29 AM, J. Mark Owens <[hidden email]> wrote:

> I would be in favor of this change. As my eyes continue to get weaker, I
> have to use larger fonts resulting in a lot of the code being off the
> screen. The constant need to scroll horizontally can get irritating.
>
>
>
> ------ Original Message ------
> From: "Christopher" <[hidden email]>
> To: "accumulo-dev" <[hidden email]>
> Sent: 2/15/2018 11:24:31 PM
> Subject: [DISCUSS] Proposed formatter change: 100 char lines
>
> Primarily for accessibility reasons (screen space with a comfortable font),
>> but also to support readability for devs working on sensibly-sized
>> screens,
>> I want to change our formatter to format with 100 char line length instead
>> of its current 160.
>>
>> Many of our files need to be reformatted anyway, because the current
>> formatter is configured incorrectly for Java 8 lambda syntax and needs to
>> be fixed, so this might be a good opportunity to make the switch.
>>
>> Also, at this point I think it is sensible to require Java 8 to build
>> Accumulo... even when building older branches. (Accumulo 1.x will still
>> support running on Java 7, of course, but Java 8 would be required to
>> build
>> it). The reason for this requirement is that in order to reduce merge
>> conflicts and merge bugs between branches, I'd like to update the
>> formatting across all branches, but the formatter which supports this
>> syntax requires Java 8 to run. The alternative to requiring Java 8 would
>> be
>> to only run the formatter when building with Java 8... and skip formatting
>> if building with Java 7, which might result in some unformatted
>> contributions, depending on the JRE version used to build.
>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Proposed formatter change: 100 char lines

Sean Busbey
In reply to this post by Christopher Tubbs-2
I'm opposed to requiring Java 8 to build on branches that we claim support
running under Java 7. Historically relying on "compile for earlier target
JDK" has just led to pain down the road when it inevitably doesn't work.

Just make it a recommendation for contributions and have our precommit
checks do the build with Java 8 to verify the formatting has already
happened.

On Thu, Feb 15, 2018 at 10:24 PM, Christopher <[hidden email]> wrote:

> Primarily for accessibility reasons (screen space with a comfortable font),
> but also to support readability for devs working on sensibly-sized screens,
> I want to change our formatter to format with 100 char line length instead
> of its current 160.
>
> Many of our files need to be reformatted anyway, because the current
> formatter is configured incorrectly for Java 8 lambda syntax and needs to
> be fixed, so this might be a good opportunity to make the switch.
>
> Also, at this point I think it is sensible to require Java 8 to build
> Accumulo... even when building older branches. (Accumulo 1.x will still
> support running on Java 7, of course, but Java 8 would be required to build
> it). The reason for this requirement is that in order to reduce merge
> conflicts and merge bugs between branches, I'd like to update the
> formatting across all branches, but the formatter which supports this
> syntax requires Java 8 to run. The alternative to requiring Java 8 would be
> to only run the formatter when building with Java 8... and skip formatting
> if building with Java 7, which might result in some unformatted
> contributions, depending on the JRE version used to build.
>



--
busbey
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Proposed formatter change: 100 char lines

Josh Elser-2
+1 to not changing min-Java on the release lines that supported Java 7.

Let's just cease activity on these branches instead :)

On 2/16/18 9:55 AM, Sean Busbey wrote:

> I'm opposed to requiring Java 8 to build on branches that we claim support
> running under Java 7. Historically relying on "compile for earlier target
> JDK" has just led to pain down the road when it inevitably doesn't work.
>
> Just make it a recommendation for contributions and have our precommit
> checks do the build with Java 8 to verify the formatting has already
> happened.
>
> On Thu, Feb 15, 2018 at 10:24 PM, Christopher <[hidden email]> wrote:
>
>> Primarily for accessibility reasons (screen space with a comfortable font),
>> but also to support readability for devs working on sensibly-sized screens,
>> I want to change our formatter to format with 100 char line length instead
>> of its current 160.
>>
>> Many of our files need to be reformatted anyway, because the current
>> formatter is configured incorrectly for Java 8 lambda syntax and needs to
>> be fixed, so this might be a good opportunity to make the switch.
>>
>> Also, at this point I think it is sensible to require Java 8 to build
>> Accumulo... even when building older branches. (Accumulo 1.x will still
>> support running on Java 7, of course, but Java 8 would be required to build
>> it). The reason for this requirement is that in order to reduce merge
>> conflicts and merge bugs between branches, I'd like to update the
>> formatting across all branches, but the formatter which supports this
>> syntax requires Java 8 to run. The alternative to requiring Java 8 would be
>> to only run the formatter when building with Java 8... and skip formatting
>> if building with Java 7, which might result in some unformatted
>> contributions, depending on the JRE version used to build.
>>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] Proposed formatter change: 100 char lines

Christopher Tubbs-2
FWIW, we use the animal-sniffer plugin to verify full Java 7 compatibility
to avoid those historical issues with simply setting the target JDK. But, I
also understand if that plugin does not instill sufficient confidence for
Java 7 compatibility. I can do the format-only-in-jdk8-profile strategy, no
problem.

I'm also in favor of ceasing activity on these branches, but as long as we
are doing bug fixes from them, I think the build tooling should also be
well-maintained to make sure releasing is smooth and produces high quality
build artifacts. I'm less concerned about the 1.7 branch... especially if
we're expecting 1.7.4 to be the last 1.7 release. I can postpone any
significant formatting change until after the 1.7.4 release, if it helps
simplify things.

On Fri, Feb 16, 2018 at 10:13 AM Josh Elser <[hidden email]> wrote:

> +1 to not changing min-Java on the release lines that supported Java 7.
>
> Let's just cease activity on these branches instead :)
>
> On 2/16/18 9:55 AM, Sean Busbey wrote:
> > I'm opposed to requiring Java 8 to build on branches that we claim
> support
> > running under Java 7. Historically relying on "compile for earlier target
> > JDK" has just led to pain down the road when it inevitably doesn't work.
> >
> > Just make it a recommendation for contributions and have our precommit
> > checks do the build with Java 8 to verify the formatting has already
> > happened.
> >
> > On Thu, Feb 15, 2018 at 10:24 PM, Christopher <[hidden email]>
> wrote:
> >
> >> Primarily for accessibility reasons (screen space with a comfortable
> font),
> >> but also to support readability for devs working on sensibly-sized
> screens,
> >> I want to change our formatter to format with 100 char line length
> instead
> >> of its current 160.
> >>
> >> Many of our files need to be reformatted anyway, because the current
> >> formatter is configured incorrectly for Java 8 lambda syntax and needs
> to
> >> be fixed, so this might be a good opportunity to make the switch.
> >>
> >> Also, at this point I think it is sensible to require Java 8 to build
> >> Accumulo... even when building older branches. (Accumulo 1.x will still
> >> support running on Java 7, of course, but Java 8 would be required to
> build
> >> it). The reason for this requirement is that in order to reduce merge
> >> conflicts and merge bugs between branches, I'd like to update the
> >> formatting across all branches, but the formatter which supports this
> >> syntax requires Java 8 to run. The alternative to requiring Java 8
> would be
> >> to only run the formatter when building with Java 8... and skip
> formatting
> >> if building with Java 7, which might result in some unformatted
> >> contributions, depending on the JRE version used to build.
> >>
> >
> >
> >
>