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

Stefan Reinauer stepan at coresystems.de
Tue Apr 8 02:10:34 CEST 2008


Clearly, this is one of those cases simple enough that everybody can 
have an opinion and an endless thread of non-sense is likely to be the 
result. I beg your pardon that I don't keep out of this ;-)

David Hendricks wrote:
> Zuh? Whitespace issues are generally so insignificant that if it 
> doesn't get fixed when the code is fixed then the whitespace issues 
> might never get revisited. They're also tedious to keep track of. And 
> for a large patches the follow-up whitespace code review can fall out 
> of sync with the actual code review and make resolving conflicts 
> between the original patch and follow-up whitespace patch prone to 
> error. And in case we have to cherrypick changes to rollback it 
> doesn't make sense to have to worry about a separate patch for 
> whitespace fixes.
>
> Just my $0.02.
Absolutely.

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. 
When working on code, I fix whitespace on the fly, if I do at all. I 
never put in a separate round for just fixing whitespace because that's 
just a bogus way of spending precious time. So is seperating the 
whitespace fixes from the rest of the code after working on it.

So, guys, please. If Marc saves half an hour by not having to seperate 
the patches, and every reviewing coreboot developer has to spend 5 
minutes more during the review process, I am all for it to just check 
this in as it is. We're smart people and can separate the wheat from the 
chaff without being all too confused. If it happens to be viable, yes, 
please try to make it like that, but NACKing a patch for that reason is 
unacceptable.

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.

All the best

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080407/03dd9692/attachment.sig>


More information about the coreboot mailing list