[coreboot] [patch][v2] AMD Fam10 memory controller updates

[Russell Whitaker]

On Tue, 8 Apr 2008, Peter Stuge wrote:

> On Mon, Apr 07, 2008 at 03:56:33PM -0600, Marc Jones wrote:
>> Peter Stuge wrote:
>>>> Do you have concerns that the patch is confused by the changes
>>>
>>> Yes, I think it is.
>>>
>>>> or are you just naking on principal?
>>>
>>> No, not really.
>>>
>>>
>>> This is how I reason:
>>
>> So, yes, you are naking on principal.
>
> It was sort-of a trick question since the principle applies when I
> think whitespace changes cause confusion.
>
> I NAKed because I thought the whitespace changes were too confusing
> in this patch, not because of the principle.
>
>
> On Mon, Apr 07, 2008 at 05:10:34PM -0700, Stefan Reinauer wrote:
>> While in theory I agree with Peter that separating whitespace is a
>> lot easier to read, I don't believe this is really a practical
>> approach.
>
> Sure it is, just don't change whitespace until the code changes are
> done.
>
>
>> When working on code, I fix whitespace on the fly, if I do at all.
>
> My point is that it is much better for patch readability and long
> term repo quality if whitespace is not changed at all while making
> changes to code.
>
>
>> I never put in a separate round for just fixing whitespace because
>> that's just a bogus way of spending precious time.
>
> Me neither. I don't want to change whitespace at all. Sometimes I
> will actually play around with comments and whitespace during my
> thought process but I review and restore forgotten changes before
> sending a patch.
>
>
>> So is seperating the whitespace fixes from the rest of the code
>> after working on it.
>
> Yes, I completely agree. Improvements done after the fact indeed
> waste some time but also lead to higher quality source.
>
> Personally I don't care at all about whitespace, but I do care
> that changes in whitespace is mixed with changes in code.
>
> I am the first to admit that following any coding style will be at
> least slightly annoying unless it happens to be your own style. :)
>
>
>> If it happens to be viable, yes, please try to make it like that,
>> but NACKing a patch for that reason is unacceptable.
>
> Hm, I should NAK patches that I have reviewed and don't want
> committed, right?
>
>
>> Therefore I say Acked-by: Stefan Reinauer <stepan at coresystems.de>
>> for that patch as it is now, and I hope we can keep this
>> low-profile.
>
Why not add the -b option when using diff?
(same as --ignore-space-change )
See man diff for other useful options.

my .02
   Russ