Whitespace cleanups in drm/i915

Whitespace cleanups in drm/i915

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]


I've got this nice patch from Akshay Joshi that removes almost all of
the checkpatch.pl warnings from drm/i915. If I don't merge it now, it's
going to go stale and be useless; if I merge it only to drm-intel-next,
it will be the source of endless conflicts.

However, it's a huge patch (yes, the code was rather sloppy), and
doesn't exactly fit into the "critical patches only please" mode of the
current stage of 3.1 development.

I've checked the patch very carefully, using the obvious git diff -b to
make sure it really doesn't touch anything but whitespace, but also
using objdump -s to compare the output of the compiler. There were no
differences found with git-diff -b. The only differences found by
objdump are two whitespace changes in some debug output messages in
intel_bios.c.

I think I have three choices:

 1) merge the patch and expect complaints from upstream

 2) thank Akshay for his good intentions, discard the patch and hope
    that he feels motivated enough to do it all over again in time for
    the 3.2 merge window.

 3) thank Akshay for his good intentions and leave the code as-is,
    forever to ease back-porting of fixes to older kernel versions.

Frankly, if we're ever going to merge whitespace fixups, this would be a
pretty darn good time; drm-intel-fixes and drm-intel-next are in-sync as
I haven't started pulling 3.2 code into -next.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Whitespace cleanups in drm/i915

[Daniel Vetter]

On Thu, Sep 15, 2011 at 06:37:33PM -0700, Keith Packard wrote:
> I've got this nice patch from Akshay Joshi that removes almost all of
> the checkpatch.pl warnings from drm/i915. If I don't merge it now, it's
> going to go stale and be useless; if I merge it only to drm-intel-next,
> it will be the source of endless conflicts.
> 
> However, it's a huge patch (yes, the code was rather sloppy), and
> doesn't exactly fit into the "critical patches only please" mode of the
> current stage of 3.1 development.
> 
> I've checked the patch very carefully, using the obvious git diff -b to
> make sure it really doesn't touch anything but whitespace, but also
> using objdump -s to compare the output of the compiler. There were no
> differences found with git-diff -b. The only differences found by
> objdump are two whitespace changes in some debug output messages in
> intel_bios.c.
> 
> I think I have three choices:
> 
>  1) merge the patch and expect complaints from upstream
> 
>  2) thank Akshay for his good intentions, discard the patch and hope
>     that he feels motivated enough to do it all over again in time for
>     the 3.2 merge window.
> 
>  3) thank Akshay for his good intentions and leave the code as-is,
>     forever to ease back-porting of fixes to older kernel versions.
> 
> Frankly, if we're ever going to merge whitespace fixups, this would be a
> pretty darn good time; drm-intel-fixes and drm-intel-next are in-sync as
> I haven't started pulling 3.2 code into -next.

Well, I've just started to build up patches, so my current queue is tiny
;-)

Otoh massive whitespace changes always annoy when backporting fixes and
also when moving forward work-in-progress and old proof-of-concept
patches. If you think it's really worth it, go for it. Otherwise I think
strictly enforcing checkpatch compliance (and sparse-cleanliness, while
we're at it) going forward should get us there pretty quickly (we have
some code-churn after all). And then we could fix up the remaining code in
1-2 releases or so - by then that patch should only touch code that's in
maintaince mode for older hw, hopefully.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: Whitespace cleanups in drm/i915

[Ben Widawsky]

On Thu, 15 Sep 2011 18:37:33 -0700
"Keith Packard" <keithp@keithp.com> wrote:

> 
> I've got this nice patch from Akshay Joshi that removes almost all of
> the checkpatch.pl warnings from drm/i915. If I don't merge it now,
> it's going to go stale and be useless; if I merge it only to
> drm-intel-next, it will be the source of endless conflicts.
> 
> However, it's a huge patch (yes, the code was rather sloppy), and
> doesn't exactly fit into the "critical patches only please" mode of
> the current stage of 3.1 development.
> 
> I've checked the patch very carefully, using the obvious git diff -b
> to make sure it really doesn't touch anything but whitespace, but also
> using objdump -s to compare the output of the compiler. There were no
> differences found with git-diff -b. The only differences found by
> objdump are two whitespace changes in some debug output messages in
> intel_bios.c.
> 
> I think I have three choices:
> 
>  1) merge the patch and expect complaints from upstream
> 
>  2) thank Akshay for his good intentions, discard the patch and hope
>     that he feels motivated enough to do it all over again in time for
>     the 3.2 merge window.
> 
>  3) thank Akshay for his good intentions and leave the code as-is,
>     forever to ease back-porting of fixes to older kernel versions.
> 
> Frankly, if we're ever going to merge whitespace fixups, this would
> be a pretty darn good time; drm-intel-fixes and drm-intel-next are
> in-sync as I haven't started pulling 3.2 code into -next.
> 

Not that you asked for a vote, but I vote either 1, or 3. In that order.
Let's either do it now, or never, and have the code slowly migrate over time
(kind of like Daniel said).

My only concern is the 80 characters per line rule. I'd hate to see some of
those nice readable lines go just because it's > 80.

Ben

Re: Whitespace cleanups in drm/i915

[Akshay Joshi]

On Sat, Sep 17, 2011 at 4:17 PM, Eugeni Dodonov <eugeni@dodonov.net> wrote:
> On Thu, Sep 15, 2011 at 22:37, Keith Packard <keithp@keithp.com> wrote:
>>
>> I've got this nice patch from Akshay Joshi that removes almost all of
>> the checkpatch.pl warnings from drm/i915. If I don't merge it now, it's
>> going to go stale and be useless; if I merge it only to drm-intel-next,
>> it will be the source of endless conflicts.
>>
>> However, it's a huge patch (yes, the code was rather sloppy), and
>> doesn't exactly fit into the "critical patches only please" mode of the
>> current stage of 3.1 development.
>>
>> I've checked the patch very carefully, using the obvious git diff -b to
>> make sure it really doesn't touch anything but whitespace, but also
>> using objdump -s to compare the output of the compiler. There were no
>> differences found with git-diff -b. The only differences found by
>> objdump are two whitespace changes in some debug output messages in
>> intel_bios.c.
>>
>> I think I have three choices:
>>
>>  1) merge the patch and expect complaints from upstream
>>
>>  2) thank Akshay for his good intentions, discard the patch and hope
>>    that he feels motivated enough to do it all over again in time for
>>    the 3.2 merge window.
>>
>>  3) thank Akshay for his good intentions and leave the code as-is,
>>    forever to ease back-porting of fixes to older kernel versions.
>>
>> Frankly, if we're ever going to merge whitespace fixups, this would be a
>> pretty darn good time; drm-intel-fixes and drm-intel-next are in-sync as
>> I haven't started pulling 3.2 code into -next.
>
> I think that if we don´t get to push this patch now, we are unlikely to do
> it in nearby future. And such kind of cleanup is a nice thing to have.
> So I´d vote for option 1, and then 3, in this order.
> --
> Eugeni Dodonov
>
>

Have we reached a consensus on this? Just curious.

Akshay

Re: Whitespace cleanups in drm/i915

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 230 bytes --]

On Wed, 21 Sep 2011 16:56:12 -0400, Akshay Joshi <me@akshayjoshi.com> wrote:

> Have we reached a consensus on this? Just curious.

Your patch was merged to Dave Airlie's drm-core-next branch.

-- 
keith.packard@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]