Are whitespace changes in unrelated lines acceptable?

[Zoltan Ivanfi <zi...@cloudera.com>]

Hi,

The last three pull requests I reviewed included removal of trailing spaces
in unrelated lines of the affected files. I was wondering whether we are
fine with such changes.

These trailing spaces should not have been committed in the first place,
but removing them later adds unnecessary clutter to the file history and
can cause problems when cherry-picking/merging/rebasing/blaming. One more
thing to take into consideration is that when someone has a change with
both code changes and unrelated whitespace changes, removing the latter is
only possible via tedious manual work (as far as I know), so I am somewhat
reluctant to request that.

What is the preferred approach here?

Thanks,

Zoltan

Re: Are whitespace changes in unrelated lines acceptable?

[Ryan Blue <rb...@netflix.com.INVALID>]

I generally avoid whitespace changes that aren't on lines with other
changes. It may be annoying to revert them, but it does make maintenance
easier.

If a contributor wants to clean up whitespace, then I'd recommend a
whitespace-only commit. That way, you always know that if there are
conflicts with it there is an easy solution to merging.

rb

On Mon, Nov 6, 2017 at 1:53 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:

> Hi,
>
> The last three pull requests I reviewed included removal of trailing spaces
> in unrelated lines of the affected files. I was wondering whether we are
> fine with such changes.
>
> These trailing spaces should not have been committed in the first place,
> but removing them later adds unnecessary clutter to the file history and
> can cause problems when cherry-picking/merging/rebasing/blaming. One more
> thing to take into consideration is that when someone has a change with
> both code changes and unrelated whitespace changes, removing the latter is
> only possible via tedious manual work (as far as I know), so I am somewhat
> reluctant to request that.
>
> What is the preferred approach here?
>
> Thanks,
>
> Zoltan
>



-- 
Ryan Blue
Software Engineer
Netflix