You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Ted Yu <yu...@gmail.com> on 2011/08/27 15:51:01 UTC
removing trailing whitespace in patch
Hi,
A big portion of review is on removing trailing whitespace in patch.
I think contributors can help reduce the amount of time spent in this
trivial task.
The following description is for Mac OS X, but it is easy to customize for
your OS. See
http://stackoverflow.com/questions/149057/how-to-removing-trailing-whitespace-of-all-files-recursively
1. Generate patch as usual (e.g. 3900.addendum)
2. Use the following command on patch:
sed -i .bak -E 's/ *$//g' 3900.addendum
3. The above operation would remove trailing whitespaces on lines not
changed by the patch. So committer needs to use --ignore-whitespace option
when applying the patch.
Your comments/ideas are welcome.
Re: removing trailing whitespace in patch
Posted by Stack <sa...@gmail.com>.
I blame fit for starting this white space fetish
On Aug 27, 2011, at 18:11, Ted Yu <yu...@gmail.com> wrote:
> If you look at:
> https://reviews.apache.org/r/1668/diff/1/#index_header
>
> You would see that people other than me worry about trailing whitespace :-)
>
> On Sat, Aug 27, 2011 at 6:06 PM, Todd Lipcon <to...@cloudera.com> wrote:
>
>> I've never understood the issue - why do people care so much about
>> trailing whitespace?
>>
>> I'm picky about whitespace/indentation/style since it makes code
>> easier to read, but whitespace at end of line is invisible!
>>
>> -Todd
>>
>> On Sat, Aug 27, 2011 at 6:51 AM, Ted Yu <yu...@gmail.com> wrote:
>>> Hi,
>>> A big portion of review is on removing trailing whitespace in patch.
>>> I think contributors can help reduce the amount of time spent in this
>>> trivial task.
>>> The following description is for Mac OS X, but it is easy to customize
>> for
>>> your OS. See
>>>
>> http://stackoverflow.com/questions/149057/how-to-removing-trailing-whitespace-of-all-files-recursively
>>>
>>> 1. Generate patch as usual (e.g. 3900.addendum)
>>> 2. Use the following command on patch:
>>> sed -i .bak -E 's/ *$//g' 3900.addendum
>>> 3. The above operation would remove trailing whitespaces on lines not
>>> changed by the patch. So committer needs to use --ignore-whitespace
>> option
>>> when applying the patch.
>>>
>>> Your comments/ideas are welcome.
>>>
>>
>>
>>
>> --
>> Todd Lipcon
>> Software Engineer, Cloudera
>>
Re: removing trailing whitespace in patch
Posted by Stack <st...@duboce.net>.
Thats me. In that review I go overboard on the ws warnings. Usually
I don't even notice them but review board does ugly red whenever
extraneous spaces. The patch under review was particularly gratuitous
ws'ing so I was trying to make point that contributor might want to
pay a little more attention here.
I'd not hold up a patch because of profligate white spacing if the
rest of the contrib did the job. Committers can do the cleanup before
commit. I'm of the mind that contributing to hbase should be as
frictionless as possible.
St.Ack
On Sat, Aug 27, 2011 at 6:11 PM, Ted Yu <yu...@gmail.com> wrote:
> If you look at:
> https://reviews.apache.org/r/1668/diff/1/#index_header
>
> You would see that people other than me worry about trailing whitespace :-)
>
> On Sat, Aug 27, 2011 at 6:06 PM, Todd Lipcon <to...@cloudera.com> wrote:
>
>> I've never understood the issue - why do people care so much about
>> trailing whitespace?
>>
>> I'm picky about whitespace/indentation/style since it makes code
>> easier to read, but whitespace at end of line is invisible!
>>
>> -Todd
>>
>> On Sat, Aug 27, 2011 at 6:51 AM, Ted Yu <yu...@gmail.com> wrote:
>> > Hi,
>> > A big portion of review is on removing trailing whitespace in patch.
>> > I think contributors can help reduce the amount of time spent in this
>> > trivial task.
>> > The following description is for Mac OS X, but it is easy to customize
>> for
>> > your OS. See
>> >
>> http://stackoverflow.com/questions/149057/how-to-removing-trailing-whitespace-of-all-files-recursively
>> >
>> > 1. Generate patch as usual (e.g. 3900.addendum)
>> > 2. Use the following command on patch:
>> > sed -i .bak -E 's/ *$//g' 3900.addendum
>> > 3. The above operation would remove trailing whitespaces on lines not
>> > changed by the patch. So committer needs to use --ignore-whitespace
>> option
>> > when applying the patch.
>> >
>> > Your comments/ideas are welcome.
>> >
>>
>>
>>
>> --
>> Todd Lipcon
>> Software Engineer, Cloudera
>>
>
Re: removing trailing whitespace in patch
Posted by Ted Yu <yu...@gmail.com>.
If you look at:
https://reviews.apache.org/r/1668/diff/1/#index_header
You would see that people other than me worry about trailing whitespace :-)
On Sat, Aug 27, 2011 at 6:06 PM, Todd Lipcon <to...@cloudera.com> wrote:
> I've never understood the issue - why do people care so much about
> trailing whitespace?
>
> I'm picky about whitespace/indentation/style since it makes code
> easier to read, but whitespace at end of line is invisible!
>
> -Todd
>
> On Sat, Aug 27, 2011 at 6:51 AM, Ted Yu <yu...@gmail.com> wrote:
> > Hi,
> > A big portion of review is on removing trailing whitespace in patch.
> > I think contributors can help reduce the amount of time spent in this
> > trivial task.
> > The following description is for Mac OS X, but it is easy to customize
> for
> > your OS. See
> >
> http://stackoverflow.com/questions/149057/how-to-removing-trailing-whitespace-of-all-files-recursively
> >
> > 1. Generate patch as usual (e.g. 3900.addendum)
> > 2. Use the following command on patch:
> > sed -i .bak -E 's/ *$//g' 3900.addendum
> > 3. The above operation would remove trailing whitespaces on lines not
> > changed by the patch. So committer needs to use --ignore-whitespace
> option
> > when applying the patch.
> >
> > Your comments/ideas are welcome.
> >
>
>
>
> --
> Todd Lipcon
> Software Engineer, Cloudera
>
Re: removing trailing whitespace in patch
Posted by Todd Lipcon <to...@cloudera.com>.
I've never understood the issue - why do people care so much about
trailing whitespace?
I'm picky about whitespace/indentation/style since it makes code
easier to read, but whitespace at end of line is invisible!
-Todd
On Sat, Aug 27, 2011 at 6:51 AM, Ted Yu <yu...@gmail.com> wrote:
> Hi,
> A big portion of review is on removing trailing whitespace in patch.
> I think contributors can help reduce the amount of time spent in this
> trivial task.
> The following description is for Mac OS X, but it is easy to customize for
> your OS. See
> http://stackoverflow.com/questions/149057/how-to-removing-trailing-whitespace-of-all-files-recursively
>
> 1. Generate patch as usual (e.g. 3900.addendum)
> 2. Use the following command on patch:
> sed -i .bak -E 's/ *$//g' 3900.addendum
> 3. The above operation would remove trailing whitespaces on lines not
> changed by the patch. So committer needs to use --ignore-whitespace option
> when applying the patch.
>
> Your comments/ideas are welcome.
>
--
Todd Lipcon
Software Engineer, Cloudera