Dangling whitespaces in xen code

Dangling whitespaces in xen code

[Volodymyr Babchuk]

Hello all,

During xen hacking I often encounter white spaces at EOLs. It is quite 
annoying to see red marker in, say, xen/arch/arm/domctl.c:25

I used sed to create patch that removes all such whitespaces. Command is 
simple:

# find xen -name "*.[ch]" | xargs sed -i "s/[[:space:]]*$//g"

Problem is that patch is quite big:

  285 files changed, 1463 insertions(+), 1463 deletions(-)

So what is the best way to publish this fix? I can just send it to ML as 
one patch. There is no functional changes, so should I add maintainers?

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Dangling whitespaces in xen code

[Andrew Cooper]

On 29/08/18 17:11, Volodymyr Babchuk wrote:
> Hello all,
>
> During xen hacking I often encounter white spaces at EOLs. It is quite
> annoying to see red marker in, say, xen/arch/arm/domctl.c:25
>
> I used sed to create patch that removes all such whitespaces. Command
> is simple:
>
> # find xen -name "*.[ch]" | xargs sed -i "s/[[:space:]]*$//g"
>
> Problem is that patch is quite big:
>
>  285 files changed, 1463 insertions(+), 1463 deletions(-)
>
> So what is the best way to publish this fix? I can just send it to ML
> as one patch. There is no functional changes, so should I add
> maintainers?
>

There are more files than that.  By my count, its:

 307 files changed, 1586 insertions(+), 1586 deletions(-)

for the Xen subtree, and

 725 files changed, 5183 insertions(+), 5183 deletions(-)

for the entire tree.

In the past, it has been the view that fixing this all in one go might
be too intrusive to people developing series.  Then again, since the
last time this was discussed, we've switched from hg to git as a base
VCS, and git rebase has no issues coping with trailing whitespace changes.

We've been fixing it as we go, but it is very slow going.  I've got half
a mind to suggest that we just commit a single fix and call it done...

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Dangling whitespaces in xen code

[Volodymyr Babchuk]

Hi Andrew,

On 29.08.18 19:22, Andrew Cooper wrote:
> On 29/08/18 17:11, Volodymyr Babchuk wrote:
>> Hello all,
>>
>> During xen hacking I often encounter white spaces at EOLs. It is quite
>> annoying to see red marker in, say, xen/arch/arm/domctl.c:25
>>
>> I used sed to create patch that removes all such whitespaces. Command
>> is simple:
>>
>> # find xen -name "*.[ch]" | xargs sed -i "s/[[:space:]]*$//g"
>>
>> Problem is that patch is quite big:
>>
>>   285 files changed, 1463 insertions(+), 1463 deletions(-)
>>
>> So what is the best way to publish this fix? I can just send it to ML
>> as one patch. There is no functional changes, so should I add
>> maintainers?
>>
> 
> There are more files than that.  By my count, its:
> 
>   307 files changed, 1586 insertions(+), 1586 deletions(-)
I tried to fix only *.c and *.h files. With COPYINGs, READMEs, 
makefiles, and so on I got

  309 files changed, 1599 insertions(+), 1599 deletions(-)


> In the past, it has been the view that fixing this all in one go might
> be too intrusive to people developing series.  Then again, since the
> last time this was discussed, we've switched from hg to git as a base
> VCS, and git rebase has no issues coping with trailing whitespace changes.

Also there is --ignore-space-change and --ignore-whitespace, so
developers and maintainers will be able to rebase changes without much 
effort.

> We've been fixing it as we go, but it is very slow going.  I've got half
> a mind to suggest that we just commit a single fix and call it done...
I can't find a better solution. At first I though to fix them manually 
in places where I see them. But then, why should I include fixes for 
some files and not include for others? How to chose which files should 
be fixed and which - not? So, I think it is better to fix all in single 
commit.

Another solution that I can see is to ask maintainers to provide patches 
for own subsystems.

-- 
Volodymyr Babchuk

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Dangling whitespaces in xen code

[Andrew Cooper]

On 29/08/18 18:01, Volodymyr Babchuk wrote:
> Hi Andrew,
>
> On 29.08.18 19:22, Andrew Cooper wrote:
>> On 29/08/18 17:11, Volodymyr Babchuk wrote:
>>> Hello all,
>>>
>>> During xen hacking I often encounter white spaces at EOLs. It is quite
>>> annoying to see red marker in, say, xen/arch/arm/domctl.c:25
>>>
>>> I used sed to create patch that removes all such whitespaces. Command
>>> is simple:
>>>
>>> # find xen -name "*.[ch]" | xargs sed -i "s/[[:space:]]*$//g"
>>>
>>> Problem is that patch is quite big:
>>>
>>>   285 files changed, 1463 insertions(+), 1463 deletions(-)
>>>
>>> So what is the best way to publish this fix? I can just send it to ML
>>> as one patch. There is no functional changes, so should I add
>>> maintainers?
>>>
>>
>> There are more files than that.  By my count, its:
>>
>>   307 files changed, 1586 insertions(+), 1586 deletions(-)
> I tried to fix only *.c and *.h files. With COPYINGs, READMEs,
> makefiles, and so on I got
>
>  309 files changed, 1599 insertions(+), 1599 deletions(-)
>
>
>> In the past, it has been the view that fixing this all in one go might
>> be too intrusive to people developing series.  Then again, since the
>> last time this was discussed, we've switched from hg to git as a base
>> VCS, and git rebase has no issues coping with trailing whitespace
>> changes.
>
> Also there is --ignore-space-change and --ignore-whitespace, so
> developers and maintainers will be able to rebase changes without much
> effort.
>
>> We've been fixing it as we go, but it is very slow going.  I've got half
>> a mind to suggest that we just commit a single fix and call it done...
> I can't find a better solution. At first I though to fix them manually
> in places where I see them. But then, why should I include fixes for
> some files and not include for others? How to chose which files should
> be fixed and which - not? So, I think it is better to fix all in
> single commit.
>
> Another solution that I can see is to ask maintainers to provide
> patches for own subsystems.

Getting the fix into the tree is trivial, and most easily done by one of
the committers, if the community can agree that its a sensible thing to do.

FWIW, +1 to getting it fixed and removing the problem completely.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: Dangling whitespaces in xen code

[Jan Beulich]

>>> On 29.08.18 at 19:09, <andrew.cooper3@citrix.com> wrote:
> On 29/08/18 18:01, Volodymyr Babchuk wrote:
>> Hi Andrew,
>>
>> On 29.08.18 19:22, Andrew Cooper wrote:
>>> On 29/08/18 17:11, Volodymyr Babchuk wrote:
>>>> Hello all,
>>>>
>>>> During xen hacking I often encounter white spaces at EOLs. It is quite
>>>> annoying to see red marker in, say, xen/arch/arm/domctl.c:25
>>>>
>>>> I used sed to create patch that removes all such whitespaces. Command
>>>> is simple:
>>>>
>>>> # find xen -name "*.[ch]" | xargs sed -i "s/[[:space:]]*$//g"
>>>>
>>>> Problem is that patch is quite big:
>>>>
>>>>   285 files changed, 1463 insertions(+), 1463 deletions(-)
>>>>
>>>> So what is the best way to publish this fix? I can just send it to ML
>>>> as one patch. There is no functional changes, so should I add
>>>> maintainers?
>>>>
>>>
>>> There are more files than that.  By my count, its:
>>>
>>>   307 files changed, 1586 insertions(+), 1586 deletions(-)
>> I tried to fix only *.c and *.h files. With COPYINGs, READMEs,
>> makefiles, and so on I got
>>
>>  309 files changed, 1599 insertions(+), 1599 deletions(-)
>>
>>
>>> In the past, it has been the view that fixing this all in one go might
>>> be too intrusive to people developing series.  Then again, since the
>>> last time this was discussed, we've switched from hg to git as a base
>>> VCS, and git rebase has no issues coping with trailing whitespace
>>> changes.
>>
>> Also there is --ignore-space-change and --ignore-whitespace, so
>> developers and maintainers will be able to rebase changes without much
>> effort.
>>
>>> We've been fixing it as we go, but it is very slow going.  I've got half
>>> a mind to suggest that we just commit a single fix and call it done...
>> I can't find a better solution. At first I though to fix them manually
>> in places where I see them. But then, why should I include fixes for
>> some files and not include for others? How to chose which files should
>> be fixed and which - not? So, I think it is better to fix all in
>> single commit.
>>
>> Another solution that I can see is to ask maintainers to provide
>> patches for own subsystems.
> 
> Getting the fix into the tree is trivial, and most easily done by one of
> the committers, if the community can agree that its a sensible thing to do.
> 
> FWIW, +1 to getting it fixed and removing the problem completely.

But please not with a single giant patch. The changes being trivial is
one thing, but whoever is to commit this would have to at least
convince themselves that the claim in the commit message is actually
true. Also please don't forget that not everyone is using git for their
day to day work.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel