Whitespace and patches WAS Re: [Patch] SSM support for multicast.c

[Colm MacCarthaigh <co...@stdlib.net>]

On Tue, Jan 11, 2005 at 06:25:57PM +0000, Julian Foad wrote:
> Colm MacCarthaigh wrote:
> >Patch also contains some whitespace/style cleanups my editor applied
> >(uggh) [...]
> 
> When you do a significant amount of whitespace cleanup, like in this patch, 
> it is better to send the cleanup and the functional change as two separate 
> patches.  (e.g. Load the original file, clean it, save it.  Then send the 
> diff from original to that as a cleanup patch, and the diff from that to 
> your functionally changed version as the functional patch.)  The combined 
> patch is hard to review because it has lots of irrelevant bits in it.

I'll well aware of this, and the whitespace cleanup was not intended. 
I have *counts* 5 different coding-styles to cope with here, due to
contributing to various projects, so I long ago gave up even trying to
change my style depending on what I'm writing.

Instead, I write in the one-true-way that suits me and have
editor/indent settings and scripts to take care of it for me, I doubt
I'm alone in this :) I probably compound it by maintaining my own set of
httpd patches for HEAnet, that arn't in a style anything like the Apache
style.

But anyway, which is worse? 

	1. Submitting patches that are broken (or at best semi-broken) 
	   style-wise.

	2. Submitting patches with some whitespace fixes in-line
	   (like the one I submitted).

	3. Checking first, to see if it's style-compliant, running
	   the indenter/whatever, producing a pure whitespace patch,
	   adding whatever code and then producing that patch and
	   submitting both.

I realise there's secret option 4 - just code in the Apache style - is
the ideal-case but honestly I just can't productively cope with 4-space
indents ;) Doing so usually causes me to take 10 times as long and make
twice as many non-style errors.

I never like the idea of pushing work onto others, so I realise the
badness of inline-whitespace-fixing patches - they are harder to
audit. I personally like to make sure my patches are style-correct
to lessen the load on the committers, so I don't like option 1 for
the same reason.

Option 3 seems like a compromise, but has the disadvantage of the
interesting patch becoming a compound patch, and one that isn't directly
applicable to the svn tree - something which strikes a blow to revision
control and patch religion :)

So summary; options 1-3 place slightly more work on the part of
Committers, secret option 4 places an annoying ammount of extra work on
the part of a non-committer (well, Me).

So, asking as a non-committer, what are Committers most inclined to
accept as reasonable? Of course, you can just tell me to get lost, and
stop whining is perfectly reasonable, I'm just trying really hard not to
have to cope with writing natively in the Apache style.

-- 
Colm MacCárthaigh                        Public Key: colm+pgp@stdlib.net

Re: Whitespace and patches WAS Re: [Patch] SSM support for multicast.c

[Justin Erenkrantz <ju...@erenkrantz.com>]

On Tue, Jan 11, 2005 at 11:19:25PM +0000, Colm MacCarthaigh wrote:
> But anyway, which is worse? 
> 
> 	1. Submitting patches that are broken (or at best semi-broken) 
> 	   style-wise.
> 
> 	2. Submitting patches with some whitespace fixes in-line
> 	   (like the one I submitted).
> 
> 	3. Checking first, to see if it's style-compliant, running
> 	   the indenter/whatever, producing a pure whitespace patch,
> 	   adding whatever code and then producing that patch and
> 	   submitting both.
> 
> I realise there's secret option 4 - just code in the Apache style - is
> the ideal-case but honestly I just can't productively cope with 4-space
> indents ;) Doing so usually causes me to take 10 times as long and make
> twice as many non-style errors.

As Julian implied, #3 is okay.  #4 is best.  #1 or #2 aren't going to be
accepted for the most part.  -- justin