Hi James, James Carlson p????e v p?? 26. 09. 2008 v 09:53 -0400: > Milan Jurik writes: > > The decision "what's related and what isn't" is very subjective. > > I think that's a key observation. Only the responsible engineer (with > guidance his code/design reviewers and RTI Advocate) can determine > what's an appropriate course of action -- not just for what CRs to > combine in one go, but also for the scope of a fix, testing required, > and other issues. > > Thus, I don't think "prohibit" is really appropriate or possible here. >
Strong word, yes, my apologize :-) > > Personally I would prefer in one changeset only cross=depending CRs but > > I am alone probably :-) > > I don't think you're alone on that. It's the situation-dependent > definition of "cross-depending" that I think is at issue. > > For yet another example, if I were fixing one or two CRs in a single > technical area, I'd make sure I included as many small fixes in that > area as I could (with separate, likely lower-priority CRs), even if > the issues in those other CRs are otherwise "unrelated." > > I would do this for efficiency reasons. I don't want to pester the > same list of code reviewers N separate times for each of N CRs. I How did you minimize code review by including several CRs in one webrev? They need to check the same amount of changes. Are you sure it's easier to review several intermixed changes? > don't want to waste N times the amount of effort going through testing > and filing RTIs. I don't want to cost the RTI advocate (probably the > same guy for each one) N times the effort to review each of these > separate RTIs. I certainly don't want to futz with the tools enough These I understand. > to keep these changesets separate all the way up to a "push." > As I wrote if CRs are splitted in suggested fix (bad that it isn't available on bugs.opensolaris.org) only regression hunting remains complicated a bit. So this policy should be enforced where possible. > More importantly, *not* doing this sort of bundling means that > low-priority (but annoying and customer-dissatisfaction-generating) > CRs are simply never addressed. Ever. They rot in the database, and > the code rots as a result. > So maybe we should improve QA and RTI to decrease amount of work spent there? > I think bundling like that is a good thing. People should be > encouraged to clean up the neighborhood when making fixes. If they're > not, then those "other" CRs will likely linger forever as the priority > never makes them reasonable to fix. > > You're probably right that bundling together (say) "update Perl to > version 1234" and "fix TCP retransmit bug" would be foolish. I don't > think anyone's going to do something like that, though, and thus my > previous comment. "Prohibit" is just too strong. Heck, even > discouraging bundling might be wrong, as it encourages poor behavior, > and allows the system to rot over time. > In your last 2 paragraphs you are saying that bundling CRs in one changeset is OK because it workarounds procedural pain, aren't you? Nevermind, yes, "prohibit" is strong word and we should avoid flame :-) Best regards, Milan
