Peter Memishian writes: > > > I guess the question is: which is more common: "whitespace obscures the > > change", or "whitespace is the change", combined with "if there's a > > problem with the webrev behavior, which is more likely to be noticed". > > > > Frankly, after some more thought, I sorta think the best would be to > > generate versions with and without whitespace changes and let the > > reviewer choose which to see... > > Assuming the underlying diff tools are sophisticated enough, you should > be able to have it both ways -- e.g., color all of the changed lines > (whitespace included), and then use a different color to indicate the > various characters on a given line that have changed from the original.
I think that'd be the optimal result. As for the previous question about which is more important, I think the previous poster left something out. It's not just "whitespace is the change" but "whitespace is the change *and* the change is non-trivial such that reviewers should spend time looking at it." There are many cases where whitespace is indeed the change, but the change doesn't really matter ... because it's just repairing some long-broken indenting. (Granted, that can go either way, but it seems to me that with 'wx nits' in common use, fixes are more common than breakage here.) A big -1 from me for merely removing the "-b" flag alone. That'd produce substantially worse results from my perspective, because it'd mean that, in many of the diffs I review, important changes would be buried in a sea of nothingness. It's already hard enough to wade through diffs where someone has decided to reorder all of the functions in a file for no apparent reason, or has done massive block copies of code. Adding null-effect whitespace changes to the list wouldn't be welcome. -- James Carlson, Solaris Networking <[EMAIL PROTECTED]> Sun Microsystems / 35 Network Drive 71.232W Vox +1 781 442 2084 MS UBUR02-212 / Burlington MA 01803-2757 42.496N Fax +1 781 442 1677 _______________________________________________ tools-discuss mailing list tools-discuss@opensolaris.org
