On Nov 28, 2019, at 09:05, Lars Kurth <lars.ku...@citrix.com> wrote: > > On 28/11/2019, 07:37, "Jan Beulich" <jbeul...@suse.com> wrote: > >> On 28.11.2019 14:06, Lars Kurth wrote: >> I can certainly add something on the timing , along the lines of >> * For complex series, consider the time it takes to do reviews (maybe with a >> guide of LOC per hour) and give reviewers enough time to >> * For series with design issues or large questions, try and highlight the >> key open issues in cover letters clearly and solicit feedback from key >> maintainers who can comment on the open issue. The idea is to save both the >> contributor and the reviewers time by focussing on what needs to be resolved >> * Don’t repost a series, unless all review comments are addressed >> or the reviewers asked you to do so. The problem with this is that >> this is somewhat in conflict with the "let's focus on the core >> issues and not get distracted by details early on in a review cycle". >> In other words, this can only work, if reviewers focus on major >> issues in early reviews only and do not focus on style, coding >> standards, etc. > > But this doesn't make much sense either, because then full re-reviews > need to happen anyway on later versions, to also deal with the minor > issues. For RFC kind of series omitting style and alike feedback > certainly makes sense, but as soon as a patch is non-RFC, it should > be considered good to go in by the submitter. > > OK, I think we have a disconnect between ideal and reality. > > I see two issues today > * Key maintainers don't always review RFC series [they end up at the bottom > of the priority list, even though spending time on RFCs will save time > elsewhere later]. So the effect is that then the contributor assumes there > are no major issues and ends it as a proper series > * In practice what has happened often in the past is that design, > architecture, assumption flaws are found in early versions of a series. > - This usually happens because of an oversight or because there was no > design discussion prior to the series being posted and agreed > - Common sense would dictate that the biggest benefit for both the > reviewer, the contributor and the community as a whole would be to try and > focus on such flaws and leave everything aside > - Of course there may be value in doing a detailed reviews of such a series > as there may be bits that are unaffected by such a flaw > - But there will likely be parts which are not: doing a detailed review of > such portions wastes everyone's time > > So coming back to your point. Ideally, it would be nice if we had the > capability to call out parts of a series as "problematic" and treating such > parts differently.
We may be able to reuse some "Shift Left" terminology, including citations of previous Xen code reviews to illustrate categories of design issues that can be shifted left: https://devopedia.org/shift-left Rich
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
