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

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Tue Apr 8 04:22:56 CEST 2008


On 08.04.2008 03:53, Russell Whitaker wrote:
> 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.
>   

The corresponding svn diff command looks like this:
svn diff -x -b

Regards,
Carl-Daniel




More information about the coreboot mailing list