hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stack <st...@duboce.net>
Subject Re: removing trailing whitespace in patch
Date Sun, 28 Aug 2011 04:55:54 GMT
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 <yuzhihong@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 <todd@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 <yuzhihong@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
>>
>

Mime
View raw message