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@coresystems.de for that patch as it is now, and I hope we can keep this low-profile.
All the best
Stefan