Accumulo, mail # dev - Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

David Medinets 2013-03-06, 15:23

I have a free day due to snowfall and while this is a fairly silly
rule, writing a short script to rule all the java files through sed
should be fairly painless. As part of this change, I will commit a
one-rule checkstyle.xml file which just runs this 'no spaces at end of
line' rule. Over time, more rules can be added to that align with the
Accumulo community's style guidelines.

Any objection?

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Christopher 2013-03-06, 19:57

Wouldn't that rule conflict with our formatter? I'm pretty sure our
formatter allows "blank" lines to be indented with spaces.

--
Christopher L Tubbs II
http://gravatar.com/ctubbsii
On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I have a free day due to snowfall and while this is a fairly silly
> rule, writing a short script to rule all the java files through sed
> should be fairly painless. As part of this change, I will commit a
> one-rule checkstyle.xml file which just runs this 'no spaces at end of
> line' rule. Over time, more rules can be added to that align with the
> Accumulo community's style guidelines.
>
> Any objection?

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

David Medinets 2013-03-06, 21:29

Chris, you seem to be correct. In fact, the Eclipse formatter actually
adds whitespace on blank lines.

  public static HdfsFileSystemConfigBuilder getInstance() {
   return BUILDER;
  }
<space><space>
  /**
  */

I removed the spaces show above. Then ran Source > Format. The spaces
were inserted for me.

I don't know how to make Eclipse stop adding the spaces. I tried
turning on the 'Remove trailing whitespace on all lines' option in
Java > Editor > Save Actions but that did not help.

Any ideas?

On Wed, Mar 6, 2013 at 2:57 PM, Christopher <[EMAIL PROTECTED]> wrote:
> Wouldn't that rule conflict with our formatter? I'm pretty sure our
> formatter allows "blank" lines to be indented with spaces.
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> <[EMAIL PROTECTED]> wrote:
>> I have a free day due to snowfall and while this is a fairly silly
>> rule, writing a short script to rule all the java files through sed
>> should be fairly painless. As part of this change, I will commit a
>> one-rule checkstyle.xml file which just runs this 'no spaces at end of
>> line' rule. Over time, more rules can be added to that align with the
>> Accumulo community's style guidelines.
>>
>> Any objection?

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Billie Rinaldi 2013-03-06, 21:53

Do Java -> Code Style -> Formatter -> Edit (assuming you have the Accumulo
formatter) -> Indentation, and the last checkbox is "Empty lines". Uncheck
that checkbox, Apply/OK, Apply, then export the formatter and replace
trunk/contrib/Eclipse-Accumulo-Codestyle.xml.

Or it would probably also work to change the following line to false in the
Codestyle and re-import it.
<setting id="org.eclipse.jdt.core.formatter.indent_empty_lines"
value="true"/>

Billie
On Wed, Mar 6, 2013 at 1:29 PM, David Medinets <[EMAIL PROTECTED]>wrote:

> Chris, you seem to be correct. In fact, the Eclipse formatter actually
> adds whitespace on blank lines.
>
> public static HdfsFileSystemConfigBuilder getInstance() {
> return BUILDER;
> }
> <space><space>
> /**
> */
>
> I removed the spaces show above. Then ran Source > Format. The spaces
> were inserted for me.
>
> I don't know how to make Eclipse stop adding the spaces. I tried
> turning on the 'Remove trailing whitespace on all lines' option in
> Java > Editor > Save Actions but that did not help.
>
> Any ideas?
>
> On Wed, Mar 6, 2013 at 2:57 PM, Christopher <[EMAIL PROTECTED]> wrote:
> > Wouldn't that rule conflict with our formatter? I'm pretty sure our
> > formatter allows "blank" lines to be indented with spaces.
> >
> > --
> > Christopher L Tubbs II
> > http://gravatar.com/ctubbsii
> >
> >
> > On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> > <[EMAIL PROTECTED]> wrote:
> >> I have a free day due to snowfall and while this is a fairly silly
> >> rule, writing a short script to rule all the java files through sed
> >> should be fairly painless. As part of this change, I will commit a
> >> one-rule checkstyle.xml file which just runs this 'no spaces at end of
> >> line' rule. Over time, more rules can be added to that align with the
> >> Accumulo community's style guidelines.
> >>
> >> Any objection?
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Christopher 2013-03-06, 22:57

+1 for this formatter change also.

--
Christopher L Tubbs II
http://gravatar.com/ctubbsii
On Wed, Mar 6, 2013 at 4:53 PM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
> Do Java -> Code Style -> Formatter -> Edit (assuming you have the Accumulo
> formatter) -> Indentation, and the last checkbox is "Empty lines". Uncheck
> that checkbox, Apply/OK, Apply, then export the formatter and replace
> trunk/contrib/Eclipse-Accumulo-Codestyle.xml.
>
> Or it would probably also work to change the following line to false in the
> Codestyle and re-import it.
> <setting id="org.eclipse.jdt.core.formatter.indent_empty_lines"
> value="true"/>
>
> Billie
>
>
> On Wed, Mar 6, 2013 at 1:29 PM, David Medinets <[EMAIL PROTECTED]>wrote:
>
>> Chris, you seem to be correct. In fact, the Eclipse formatter actually
>> adds whitespace on blank lines.
>>
>> public static HdfsFileSystemConfigBuilder getInstance() {
>> return BUILDER;
>> }
>> <space><space>
>> /**
>> */
>>
>> I removed the spaces show above. Then ran Source > Format. The spaces
>> were inserted for me.
>>
>> I don't know how to make Eclipse stop adding the spaces. I tried
>> turning on the 'Remove trailing whitespace on all lines' option in
>> Java > Editor > Save Actions but that did not help.
>>
>> Any ideas?
>>
>> On Wed, Mar 6, 2013 at 2:57 PM, Christopher <[EMAIL PROTECTED]> wrote:
>> > Wouldn't that rule conflict with our formatter? I'm pretty sure our
>> > formatter allows "blank" lines to be indented with spaces.
>> >
>> > --
>> > Christopher L Tubbs II
>> > http://gravatar.com/ctubbsii
>> >
>> >
>> > On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>> > <[EMAIL PROTECTED]> wrote:
>> >> I have a free day due to snowfall and while this is a fairly silly
>> >> rule, writing a short script to rule all the java files through sed
>> >> should be fairly painless. As part of this change, I will commit a
>> >> one-rule checkstyle.xml file which just runs this 'no spaces at end of
>> >> line' rule. Over time, more rules can be added to that align with the
>> >> Accumulo community's style guidelines.
>> >>
>> >> Any objection?
>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

David Medinets 2013-03-07, 01:49

Done.

On Wed, Mar 6, 2013 at 5:57 PM, Christopher <[EMAIL PROTECTED]> wrote:
> +1 for this formatter change also.
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Wed, Mar 6, 2013 at 4:53 PM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>> Do Java -> Code Style -> Formatter -> Edit (assuming you have the Accumulo
>> formatter) -> Indentation, and the last checkbox is "Empty lines". Uncheck
>> that checkbox, Apply/OK, Apply, then export the formatter and replace
>> trunk/contrib/Eclipse-Accumulo-Codestyle.xml.
>>
>> Or it would probably also work to change the following line to false in the
>> Codestyle and re-import it.
>> <setting id="org.eclipse.jdt.core.formatter.indent_empty_lines"
>> value="true"/>
>>
>> Billie
>>
>>
>> On Wed, Mar 6, 2013 at 1:29 PM, David Medinets <[EMAIL PROTECTED]>wrote:
>>
>>> Chris, you seem to be correct. In fact, the Eclipse formatter actually
>>> adds whitespace on blank lines.
>>>
>>> public static HdfsFileSystemConfigBuilder getInstance() {
>>> return BUILDER;
>>> }
>>> <space><space>
>>> /**
>>> */
>>>
>>> I removed the spaces show above. Then ran Source > Format. The spaces
>>> were inserted for me.
>>>
>>> I don't know how to make Eclipse stop adding the spaces. I tried
>>> turning on the 'Remove trailing whitespace on all lines' option in
>>> Java > Editor > Save Actions but that did not help.
>>>
>>> Any ideas?
>>>
>>> On Wed, Mar 6, 2013 at 2:57 PM, Christopher <[EMAIL PROTECTED]> wrote:
>>> > Wouldn't that rule conflict with our formatter? I'm pretty sure our
>>> > formatter allows "blank" lines to be indented with spaces.
>>> >
>>> > --
>>> > Christopher L Tubbs II
>>> > http://gravatar.com/ctubbsii
>>> >
>>> >
>>> > On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>>> > <[EMAIL PROTECTED]> wrote:
>>> >> I have a free day due to snowfall and while this is a fairly silly
>>> >> rule, writing a short script to rule all the java files through sed
>>> >> should be fairly painless. As part of this change, I will commit a
>>> >> one-rule checkstyle.xml file which just runs this 'no spaces at end of
>>> >> line' rule. Over time, more rules can be added to that align with the
>>> >> Accumulo community's style guidelines.
>>> >>
>>> >> Any objection?
>>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Billie Rinaldi 2013-03-06, 20:18

I personally would prefer if we changed our formatter to have no spaces on
blank lines. git calls these "whitespace errors" when you're applying a
patch.

Billie
On Wed, Mar 6, 2013 at 11:57 AM, Christopher <[EMAIL PROTECTED]> wrote:

> Wouldn't that rule conflict with our formatter? I'm pretty sure our
> formatter allows "blank" lines to be indented with spaces.
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> <[EMAIL PROTECTED]> wrote:
> > I have a free day due to snowfall and while this is a fairly silly
> > rule, writing a short script to rule all the java files through sed
> > should be fairly painless. As part of this change, I will commit a
> > one-rule checkstyle.xml file which just runs this 'no spaces at end of
> > line' rule. Over time, more rules can be added to that align with the
> > Accumulo community's style guidelines.
> >
> > Any objection?
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

David Medinets 2013-03-06, 20:20

+1 to change the formatter.

On Wed, Mar 6, 2013 at 3:18 PM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
> I personally would prefer if we changed our formatter to have no spaces on
> blank lines. git calls these "whitespace errors" when you're applying a
> patch.
>
> Billie
>
>
> On Wed, Mar 6, 2013 at 11:57 AM, Christopher <[EMAIL PROTECTED]> wrote:
>
>> Wouldn't that rule conflict with our formatter? I'm pretty sure our
>> formatter allows "blank" lines to be indented with spaces.
>>
>> --
>> Christopher L Tubbs II
>> http://gravatar.com/ctubbsii
>>
>>
>> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>> <[EMAIL PROTECTED]> wrote:
>> > I have a free day due to snowfall and while this is a fairly silly
>> > rule, writing a short script to rule all the java files through sed
>> > should be fairly painless. As part of this change, I will commit a
>> > one-rule checkstyle.xml file which just runs this 'no spaces at end of
>> > line' rule. Over time, more rules can be added to that align with the
>> > Accumulo community's style guidelines.
>> >
>> > Any objection?
>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Keith Turner 2013-03-07, 15:44

On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I have a free day due to snowfall and while this is a fairly silly
> rule, writing a short script to rule all the java files through sed
> should be fairly painless. As part of this change, I will commit a
> one-rule checkstyle.xml file which just runs this 'no spaces at end of
> line' rule. Over time, more rules can be added to that align with the
> Accumulo community's style guidelines.
>
> Any objection?

Whats the benefit of doing this? How will it impact merges from 1.5
to 1.6? Should this be done for thrift generated code?

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

John Vines 2013-03-07, 15:49

I've been getting unnecessary merge conflicts because of this change. At
the very least, I would like to see it reverted until we release 1.5
On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]> wrote:

> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> <[EMAIL PROTECTED]> wrote:
> > I have a free day due to snowfall and while this is a fairly silly
> > rule, writing a short script to rule all the java files through sed
> > should be fairly painless. As part of this change, I will commit a
> > one-rule checkstyle.xml file which just runs this 'no spaces at end of
> > line' rule. Over time, more rules can be added to that align with the
> > Accumulo community's style guidelines.
> >
> > Any objection?
>
> Whats the benefit of doing this? How will it impact merges from 1.5
> to 1.6? Should this be done for thrift generated code?
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Keith Turner 2013-03-07, 15:57

On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]> wrote:
> I've been getting unnecessary merge conflicts because of this change. At
> the very least, I would like to see it reverted until we release 1.5

Or maybe make the change after 1.5.1. Based on past experience, there
will likely be a good bit of merge activity from 1.5 to 1.6 until at
least the first 1.5 bug fix release.

Curious, how much extra time do this add to merging for you? I do not
have a good feeling for how well this will be handled automatically.
Did it cause conflicts for most of the edits you made in 1.5?

>
>
> On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]> wrote:
>
>> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>> <[EMAIL PROTECTED]> wrote:
>> > I have a free day due to snowfall and while this is a fairly silly
>> > rule, writing a short script to rule all the java files through sed
>> > should be fairly painless. As part of this change, I will commit a
>> > one-rule checkstyle.xml file which just runs this 'no spaces at end of
>> > line' rule. Over time, more rules can be added to that align with the
>> > Accumulo community's style guidelines.
>> >
>> > Any objection?
>>
>> Whats the benefit of doing this? How will it impact merges from 1.5
>> to 1.6? Should this be done for thrift generated code?
>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

John Vines 2013-03-07, 16:01

Since it was introduced yesterday morning, every merge I've done had at
least 1 conflict file, usually multiple. And one to many merges per file
(which is how I missed that one yesterday).
On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]> wrote:

> On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]> wrote:
> > I've been getting unnecessary merge conflicts because of this change. At
> > the very least, I would like to see it reverted until we release 1.5
>
> Or maybe make the change after 1.5.1. Based on past experience, there
> will likely be a good bit of merge activity from 1.5 to 1.6 until at
> least the first 1.5 bug fix release.
>
> Curious, how much extra time do this add to merging for you? I do not
> have a good feeling for how well this will be handled automatically.
> Did it cause conflicts for most of the edits you made in 1.5?
>
> >
> >
> > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]> wrote:
> >
> >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> >> <[EMAIL PROTECTED]> wrote:
> >> > I have a free day due to snowfall and while this is a fairly silly
> >> > rule, writing a short script to rule all the java files through sed
> >> > should be fairly painless. As part of this change, I will commit a
> >> > one-rule checkstyle.xml file which just runs this 'no spaces at end of
> >> > line' rule. Over time, more rules can be added to that align with the
> >> > Accumulo community's style guidelines.
> >> >
> >> > Any objection?
> >>
> >> Whats the benefit of doing this? How will it impact merges from 1.5
> >> to 1.6? Should this be done for thrift generated code?
> >>
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Billie Rinaldi 2013-03-07, 16:20

I don't mind if we roll it back until we stop doing so much merging.

Billie
On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:

> Since it was introduced yesterday morning, every merge I've done had at
> least 1 conflict file, usually multiple. And one to many merges per file
> (which is how I missed that one yesterday).
>
>
> On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]> wrote:
>
> > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]> wrote:
> > > I've been getting unnecessary merge conflicts because of this change.
> At
> > > the very least, I would like to see it reverted until we release 1.5
> >
> > Or maybe make the change after 1.5.1. Based on past experience, there
> > will likely be a good bit of merge activity from 1.5 to 1.6 until at
> > least the first 1.5 bug fix release.
> >
> > Curious, how much extra time do this add to merging for you? I do not
> > have a good feeling for how well this will be handled automatically.
> > Did it cause conflicts for most of the edits you made in 1.5?
> >
> > >
> > >
> > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
> wrote:
> > >
> > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> > >> <[EMAIL PROTECTED]> wrote:
> > >> > I have a free day due to snowfall and while this is a fairly silly
> > >> > rule, writing a short script to rule all the java files through sed
> > >> > should be fairly painless. As part of this change, I will commit a
> > >> > one-rule checkstyle.xml file which just runs this 'no spaces at end
> of
> > >> > line' rule. Over time, more rules can be added to that align with
> the
> > >> > Accumulo community's style guidelines.
> > >> >
> > >> > Any objection?
> > >>
> > >> Whats the benefit of doing this? How will it impact merges from 1.5
> > >> to 1.6? Should this be done for thrift generated code?
> > >>
> >
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Keith Turner 2013-03-07, 16:27

On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
> I don't mind if we roll it back until we stop doing so much merging.

I am going to experiment with revering and report the results in a bit.

>
> Billie
>
>
> On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
>
>> Since it was introduced yesterday morning, every merge I've done had at
>> least 1 conflict file, usually multiple. And one to many merges per file
>> (which is how I missed that one yesterday).
>>
>>
>> On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]> wrote:
>>
>> > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]> wrote:
>> > > I've been getting unnecessary merge conflicts because of this change.
>> At
>> > > the very least, I would like to see it reverted until we release 1.5
>> >
>> > Or maybe make the change after 1.5.1. Based on past experience, there
>> > will likely be a good bit of merge activity from 1.5 to 1.6 until at
>> > least the first 1.5 bug fix release.
>> >
>> > Curious, how much extra time do this add to merging for you? I do not
>> > have a good feeling for how well this will be handled automatically.
>> > Did it cause conflicts for most of the edits you made in 1.5?
>> >
>> > >
>> > >
>> > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
>> wrote:
>> > >
>> > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>> > >> <[EMAIL PROTECTED]> wrote:
>> > >> > I have a free day due to snowfall and while this is a fairly silly
>> > >> > rule, writing a short script to rule all the java files through sed
>> > >> > should be fairly painless. As part of this change, I will commit a
>> > >> > one-rule checkstyle.xml file which just runs this 'no spaces at end
>> of
>> > >> > line' rule. Over time, more rules can be added to that align with
>> the
>> > >> > Accumulo community's style guidelines.
>> > >> >
>> > >> > Any objection?
>> > >>
>> > >> Whats the benefit of doing this? How will it impact merges from 1.5
>> > >> to 1.6? Should this be done for thrift generated code?
>> > >>
>> >
>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

John Vines 2013-03-07, 16:24

The only changes made to trunk since that were my merges, so it should be
pretty painless to roll it back, revert that change for now, and remerge.
On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:

> I don't mind if we roll it back until we stop doing so much merging.
>
> Billie
>
>
> On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
>
> > Since it was introduced yesterday morning, every merge I've done had at
> > least 1 conflict file, usually multiple. And one to many merges per file
> > (which is how I missed that one yesterday).
> >
> >
> > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]> wrote:
> >
> > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]> wrote:
> > > > I've been getting unnecessary merge conflicts because of this change.
> > At
> > > > the very least, I would like to see it reverted until we release 1.5
> > >
> > > Or maybe make the change after 1.5.1. Based on past experience, there
> > > will likely be a good bit of merge activity from 1.5 to 1.6 until at
> > > least the first 1.5 bug fix release.
> > >
> > > Curious, how much extra time do this add to merging for you? I do not
> > > have a good feeling for how well this will be handled automatically.
> > > Did it cause conflicts for most of the edits you made in 1.5?
> > >
> > > >
> > > >
> > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
> > wrote:
> > > >
> > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> > > >> <[EMAIL PROTECTED]> wrote:
> > > >> > I have a free day due to snowfall and while this is a fairly silly
> > > >> > rule, writing a short script to rule all the java files through
> sed
> > > >> > should be fairly painless. As part of this change, I will commit a
> > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at
> end
> > of
> > > >> > line' rule. Over time, more rules can be added to that align with
> > the
> > > >> > Accumulo community's style guidelines.
> > > >> >
> > > >> > Any objection?
> > > >>
> > > >> Whats the benefit of doing this? How will it impact merges from 1.5
> > > >> to 1.6? Should this be done for thrift generated code?
> > > >>
> > >
> >
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Billie Rinaldi 2013-03-07, 16:28

Or we could apply the space changes to the 1.5 branch (not the pom change)
...

Billie
On Thu, Mar 7, 2013 at 11:24 AM, John Vines <[EMAIL PROTECTED]> wrote:

> The only changes made to trunk since that were my merges, so it should be
> pretty painless to roll it back, revert that change for now, and remerge.
>
>
> On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>
> > I don't mind if we roll it back until we stop doing so much merging.
> >
> > Billie
> >
> >
> > On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
> >
> > > Since it was introduced yesterday morning, every merge I've done had at
> > > least 1 conflict file, usually multiple. And one to many merges per
> file
> > > (which is how I missed that one yesterday).
> > >
> > >
> > > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]>
> wrote:
> > >
> > > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]>
> wrote:
> > > > > I've been getting unnecessary merge conflicts because of this
> change.
> > > At
> > > > > the very least, I would like to see it reverted until we release
> 1.5
> > > >
> > > > Or maybe make the change after 1.5.1. Based on past experience,
> there
> > > > will likely be a good bit of merge activity from 1.5 to 1.6 until at
> > > > least the first 1.5 bug fix release.
> > > >
> > > > Curious, how much extra time do this add to merging for you? I do
> not
> > > > have a good feeling for how well this will be handled automatically.
> > > > Did it cause conflicts for most of the edits you made in 1.5?
> > > >
> > > > >
> > > > >
> > > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
> > > wrote:
> > > > >
> > > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
> > > > >> <[EMAIL PROTECTED]> wrote:
> > > > >> > I have a free day due to snowfall and while this is a fairly
> silly
> > > > >> > rule, writing a short script to rule all the java files through
> > sed
> > > > >> > should be fairly painless. As part of this change, I will
> commit a
> > > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at
> > end
> > > of
> > > > >> > line' rule. Over time, more rules can be added to that align
> with
> > > the
> > > > >> > Accumulo community's style guidelines.
> > > > >> >
> > > > >> > Any objection?
> > > > >>
> > > > >> Whats the benefit of doing this? How will it impact merges from
> 1.5
> > > > >> to 1.6? Should this be done for thrift generated code?
> > > > >>
> > > >
> > >
> >
>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

David Medinets 2013-03-07, 16:55

I thought it was a safe change because I made it to trunk. Sorry about
the disruption. Please revert. This issue is not worth spending any of
your time. And we can always run the script later to remove trailing
spaces. My concern is why there was a disruption? Could I have done
something better?

On Thu, Mar 7, 2013 at 11:28 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
> Or we could apply the space changes to the 1.5 branch (not the pom change)
> ...
>
> Billie
>
>
> On Thu, Mar 7, 2013 at 11:24 AM, John Vines <[EMAIL PROTECTED]> wrote:
>
>> The only changes made to trunk since that were my merges, so it should be
>> pretty painless to roll it back, revert that change for now, and remerge.
>>
>>
>> On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>>
>> > I don't mind if we roll it back until we stop doing so much merging.
>> >
>> > Billie
>> >
>> >
>> > On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
>> >
>> > > Since it was introduced yesterday morning, every merge I've done had at
>> > > least 1 conflict file, usually multiple. And one to many merges per
>> file
>> > > (which is how I missed that one yesterday).
>> > >
>> > >
>> > > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]>
>> wrote:
>> > >
>> > > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]>
>> wrote:
>> > > > > I've been getting unnecessary merge conflicts because of this
>> change.
>> > > At
>> > > > > the very least, I would like to see it reverted until we release
>> 1.5
>> > > >
>> > > > Or maybe make the change after 1.5.1. Based on past experience,
>> there
>> > > > will likely be a good bit of merge activity from 1.5 to 1.6 until at
>> > > > least the first 1.5 bug fix release.
>> > > >
>> > > > Curious, how much extra time do this add to merging for you? I do
>> not
>> > > > have a good feeling for how well this will be handled automatically.
>> > > > Did it cause conflicts for most of the edits you made in 1.5?
>> > > >
>> > > > >
>> > > > >
>> > > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
>> > > wrote:
>> > > > >
>> > > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>> > > > >> <[EMAIL PROTECTED]> wrote:
>> > > > >> > I have a free day due to snowfall and while this is a fairly
>> silly
>> > > > >> > rule, writing a short script to rule all the java files through
>> > sed
>> > > > >> > should be fairly painless. As part of this change, I will
>> commit a
>> > > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at
>> > end
>> > > of
>> > > > >> > line' rule. Over time, more rules can be added to that align
>> with
>> > > the
>> > > > >> > Accumulo community's style guidelines.
>> > > > >> >
>> > > > >> > Any objection?
>> > > > >>
>> > > > >> Whats the benefit of doing this? How will it impact merges from
>> 1.5
>> > > > >> to 1.6? Should this be done for thrift generated code?
>> > > > >>
>> > > >
>> > >
>> >
>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Keith Turner 2013-03-07, 19:27

On Thu, Mar 7, 2013 at 11:55 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I thought it was a safe change because I made it to trunk. Sorry about
> the disruption. Please revert. This issue is not worth spending any of
> your time. And we can always run the script later to remove trailing
> spaces. My concern is why there was a disruption? Could I have done
> something better?

Experimenting with checkstyle rules that check on our current
expectations about code formating would be a good start. This
checkstyle plugin seems like a great thing and I think Accumulo should
use it. We need to come up with a strategy for introducing this into
1.6. I suspect a lot of our code does not follow our current
formatting expectations because we do not have checkstyle. So
introducing it will probably necessitate reformatting code.

Also, code formatting is something we decided as a community a while
ago. IMO this changes to code formatting should be made as a
community. Also, if we are going to change our code formatting rules
we should decide on all of the changes we want to make, and make them
at once. Making lots of little changes to the formatting rules over
time seems very disruptive (complicates merges, user patches, and
uncommitted changes in working area).

One possible way to go about this is the following :

* Send out email to dev list asking what code formatting rules
changes people would like to make (possibly have a ticket)
* After decision process settles, send another email to dev list
proposing a date to reformat code in trunk and add checkstyle

Keith

>
> On Thu, Mar 7, 2013 at 11:28 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>> Or we could apply the space changes to the 1.5 branch (not the pom change)
>> ...
>>
>> Billie
>>
>>
>> On Thu, Mar 7, 2013 at 11:24 AM, John Vines <[EMAIL PROTECTED]> wrote:
>>
>>> The only changes made to trunk since that were my merges, so it should be
>>> pretty painless to roll it back, revert that change for now, and remerge.
>>>
>>>
>>> On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>>>
>>> > I don't mind if we roll it back until we stop doing so much merging.
>>> >
>>> > Billie
>>> >
>>> >
>>> > On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
>>> >
>>> > > Since it was introduced yesterday morning, every merge I've done had at
>>> > > least 1 conflict file, usually multiple. And one to many merges per
>>> file
>>> > > (which is how I missed that one yesterday).
>>> > >
>>> > >
>>> > > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]>
>>> wrote:
>>> > >
>>> > > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]>
>>> wrote:
>>> > > > > I've been getting unnecessary merge conflicts because of this
>>> change.
>>> > > At
>>> > > > > the very least, I would like to see it reverted until we release
>>> 1.5
>>> > > >
>>> > > > Or maybe make the change after 1.5.1. Based on past experience,
>>> there
>>> > > > will likely be a good bit of merge activity from 1.5 to 1.6 until at
>>> > > > least the first 1.5 bug fix release.
>>> > > >
>>> > > > Curious, how much extra time do this add to merging for you? I do
>>> not
>>> > > > have a good feeling for how well this will be handled automatically.
>>> > > > Did it cause conflicts for most of the edits you made in 1.5?
>>> > > >
>>> > > > >
>>> > > > >
>>> > > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
>>> > > wrote:
>>> > > > >
>>> > > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>>> > > > >> <[EMAIL PROTECTED]> wrote:
>>> > > > >> > I have a free day due to snowfall and while this is a fairly
>>> silly
>>> > > > >> > rule, writing a short script to rule all the java files through
>>> > sed
>>> > > > >> > should be fairly painless. As part of this change, I will
>>> commit a
>>> > > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at
>>> > end
>>> > > of
>>> > > > >> > line' rule. Over time, more rules can be added to that align

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Keith Turner 2013-03-07, 17:13

On Thu, Mar 7, 2013 at 11:55 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I thought it was a safe change because I made it to trunk. Sorry about
> the disruption. Please revert. This issue is not worth spending any of
> your time. And we can always run the script later to remove trailing
> spaces. My concern is why there was a disruption? Could I have done
> something better?

Can you please post what the benefits of making this change are on the
ticket? I am completely clueless as to what the benefits are.
Billie mentioned there is some benefit for making this change when
using git. Why is this and are there other benefits?

>
> On Thu, Mar 7, 2013 at 11:28 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>> Or we could apply the space changes to the 1.5 branch (not the pom change)
>> ...
>>
>> Billie
>>
>>
>> On Thu, Mar 7, 2013 at 11:24 AM, John Vines <[EMAIL PROTECTED]> wrote:
>>
>>> The only changes made to trunk since that were my merges, so it should be
>>> pretty painless to roll it back, revert that change for now, and remerge.
>>>
>>>
>>> On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>>>
>>> > I don't mind if we roll it back until we stop doing so much merging.
>>> >
>>> > Billie
>>> >
>>> >
>>> > On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
>>> >
>>> > > Since it was introduced yesterday morning, every merge I've done had at
>>> > > least 1 conflict file, usually multiple. And one to many merges per
>>> file
>>> > > (which is how I missed that one yesterday).
>>> > >
>>> > >
>>> > > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]>
>>> wrote:
>>> > >
>>> > > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]>
>>> wrote:
>>> > > > > I've been getting unnecessary merge conflicts because of this
>>> change.
>>> > > At
>>> > > > > the very least, I would like to see it reverted until we release
>>> 1.5
>>> > > >
>>> > > > Or maybe make the change after 1.5.1. Based on past experience,
>>> there
>>> > > > will likely be a good bit of merge activity from 1.5 to 1.6 until at
>>> > > > least the first 1.5 bug fix release.
>>> > > >
>>> > > > Curious, how much extra time do this add to merging for you? I do
>>> not
>>> > > > have a good feeling for how well this will be handled automatically.
>>> > > > Did it cause conflicts for most of the edits you made in 1.5?
>>> > > >
>>> > > > >
>>> > > > >
>>> > > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
>>> > > wrote:
>>> > > > >
>>> > > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>>> > > > >> <[EMAIL PROTECTED]> wrote:
>>> > > > >> > I have a free day due to snowfall and while this is a fairly
>>> silly
>>> > > > >> > rule, writing a short script to rule all the java files through
>>> > sed
>>> > > > >> > should be fairly painless. As part of this change, I will
>>> commit a
>>> > > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at
>>> > end
>>> > > of
>>> > > > >> > line' rule. Over time, more rules can be added to that align
>>> with
>>> > > the
>>> > > > >> > Accumulo community's style guidelines.
>>> > > > >> >
>>> > > > >> > Any objection?
>>> > > > >>
>>> > > > >> Whats the benefit of doing this? How will it impact merges from
>>> 1.5
>>> > > > >> to 1.6? Should this be done for thrift generated code?
>>> > > > >>
>>> > > >
>>> > >
>>> >
>>>

Re: One of Checkstyle's rules is 'no spaces at end of line', anyone upset if I update the code (1.6) to follow it?

Keith Turner 2013-03-07, 16:58

On Thu, Mar 7, 2013 at 11:55 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I thought it was a safe change because I made it to trunk. Sorry about
> the disruption. Please revert. This issue is not worth spending any of
> your time. And we can always run the script later to remove trailing
> spaces. My concern is why there was a disruption? Could I have done

The problem is this type of change makes merges from 1.5 to 1.6 very
difficult. We are still making lots of bug fixes in the 1.5 branch
and merging them to trunk.

I will revert it.

> something better?
>
> On Thu, Mar 7, 2013 at 11:28 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>> Or we could apply the space changes to the 1.5 branch (not the pom change)
>> ...
>>
>> Billie
>>
>>
>> On Thu, Mar 7, 2013 at 11:24 AM, John Vines <[EMAIL PROTECTED]> wrote:
>>
>>> The only changes made to trunk since that were my merges, so it should be
>>> pretty painless to roll it back, revert that change for now, and remerge.
>>>
>>>
>>> On Thu, Mar 7, 2013 at 11:20 AM, Billie Rinaldi <[EMAIL PROTECTED]> wrote:
>>>
>>> > I don't mind if we roll it back until we stop doing so much merging.
>>> >
>>> > Billie
>>> >
>>> >
>>> > On Thu, Mar 7, 2013 at 11:01 AM, John Vines <[EMAIL PROTECTED]> wrote:
>>> >
>>> > > Since it was introduced yesterday morning, every merge I've done had at
>>> > > least 1 conflict file, usually multiple. And one to many merges per
>>> file
>>> > > (which is how I missed that one yesterday).
>>> > >
>>> > >
>>> > > On Thu, Mar 7, 2013 at 10:57 AM, Keith Turner <[EMAIL PROTECTED]>
>>> wrote:
>>> > >
>>> > > > On Thu, Mar 7, 2013 at 10:49 AM, John Vines <[EMAIL PROTECTED]>
>>> wrote:
>>> > > > > I've been getting unnecessary merge conflicts because of this
>>> change.
>>> > > At
>>> > > > > the very least, I would like to see it reverted until we release
>>> 1.5
>>> > > >
>>> > > > Or maybe make the change after 1.5.1. Based on past experience,
>>> there
>>> > > > will likely be a good bit of merge activity from 1.5 to 1.6 until at
>>> > > > least the first 1.5 bug fix release.
>>> > > >
>>> > > > Curious, how much extra time do this add to merging for you? I do
>>> not
>>> > > > have a good feeling for how well this will be handled automatically.
>>> > > > Did it cause conflicts for most of the edits you made in 1.5?
>>> > > >
>>> > > > >
>>> > > > >
>>> > > > > On Thu, Mar 7, 2013 at 10:44 AM, Keith Turner <[EMAIL PROTECTED]>
>>> > > wrote:
>>> > > > >
>>> > > > >> On Wed, Mar 6, 2013 at 10:23 AM, David Medinets
>>> > > > >> <[EMAIL PROTECTED]> wrote:
>>> > > > >> > I have a free day due to snowfall and while this is a fairly
>>> silly
>>> > > > >> > rule, writing a short script to rule all the java files through
>>> > sed
>>> > > > >> > should be fairly painless. As part of this change, I will
>>> commit a
>>> > > > >> > one-rule checkstyle.xml file which just runs this 'no spaces at
>>> > end
>>> > > of
>>> > > > >> > line' rule. Over time, more rules can be added to that align
>>> with
>>> > > the
>>> > > > >> > Accumulo community's style guidelines.
>>> > > > >> >
>>> > > > >> > Any objection?
>>> > > > >>
>>> > > > >> Whats the benefit of doing this? How will it impact merges from
>>> 1.5
>>> > > > >> to 1.6? Should this be done for thrift generated code?
>>> > > > >>
>>> > > >
>>> > >
>>> >
>>>