This would serve mostly these purposes:
1. Asking preventive review on approach of a patch, without requiring a
code-review. Often i find myself having a half done patch, but before
going on, i want to get some feedback either from module owner or my
future reviewer (especially on larger changes/projects). Since
everybody's is pretty busy, asking on IRC or by mail does not allow the
reviewer to easily organize and search these requests. Also asking
review when it is not a review has some downside: the reviewer has to
find by himself if this is a real review request or not, people reading
the bug have to distinguish by themselves again, overall progress of the
bug is less clear.
2. Asking first-pass review to someone that won't be the final reviewer
(especially useful for new contributors with less experience on our
codebase)
3. Asking feedback on a visual change to UX team, without it being an
official ui-review (for the same reasoning as the not-official review
requests in point 1). Mockups or wireframes
4. Allowing overloaded reviewers to pass some work to other less busy
reviewers, that are fine doing code reviews but have less experience
with overall vision of the change. We have reviewers with 30 or 50
reviews in queue, they could forward a feedback request to less
over-busy reviewers, and when the first-pass reviewer is satisfied the
official reviewer can do the final pass. This would reduce work load on
some over-busy person and allow to track all the requests.
Other possible solutions to the same problem could be:
A. use current review? flag. A non-final reviewer would clear the flag
instead of setting it. I think this is confusing for both the reviewers,
the drivers and the bug's readers. Plus is hard to track workflow since
real review requests are not distinguisheable from temporary review
requests.
B. have a not targeted flag, or keyword. I think this is bad because
assumes that someone tracks these requests and forward them to someone
other, knowing who is free and who is fine with certain code parts. it's
harder to track, in busy periods people would be less prone to look for
bugs waiting feedback, and it's harder to balance and organize in
personal workload.
Thoughts on this proposal?
Marco
As a note, since it's an attachment flag, it wouldn't clutter our primary UI, so I think the marginal cost is really low here, and the potential benefit is huge (growing new reviewers faster, better visibility into bug status, and ability to get quicker/earlier feedback to community patch authors).
-- Mike
Didn't we have first-review for these purposes?
No, first-review and second-review were toolkit-only flags that
existed because there was a) no SR for toolkit, but sometimes
additional review was useful and b) review could not be requested of
multiple people. First-review was a binding owner/peer review.
- Mike
https://bugzilla.mozilla.org/show_bug.cgi?id=547753 .
Gerv
Thanks for filling the bug!
Marco