On 22/01/16 02:12 AM, Gregory Szorc wrote:
On Thu, Jan 21, 2016 at 10:37 PM, Mike Connor <mcon...@mozilla.com> wrote:
Like Greg, I'm a big fan of reviewer-lands-if-ready. It's a huge
simplification of workflow, saves developers time, and lets machines do
work instead of humans. That said, I don't think we should be surprising
people or unilaterally imposing changes to their workflow. The best way to
do this is to make it simple to adopt, and demonstrably better than the old
way. Developers will migrate over time as they see the light.
I think this is an easy fix if we're willing to modify our patch
submission workflow to reflect a wider set of asks and responses. It's
more or less the same mental model as the multi-state flags we use for
tracking, there's more than one type of request, and more than one possible
response.
Simple proposal: replace review/feedback flags with a new,
multiple-requestable flag. Possible values could be:
feedback?
review?
land?
feedback+
withcomments+
review+
land+
Patch authors would be able to opt into the easier flow by setting "land?"
instead of
"review?" "review?" and "feedback?" would retain their current
definitions. Patch reviewers would be able to respond explicitly with
feedback, a conditional r+, full r+, or land+.
* Anything where land+ is set would trigger autoland.
* land+ should be set only if requested, or if the reviewer believes
that's expected
* patch authors would be allowed to autoland with review+
* withcomments+ or feedback+ would not allow autoland.
In the short term, we could experiment with this as an additional patch
flag (land?/+/-). I think the multi-state flag reflects current workflow
reality and eliminates nuance, so should be explored.
We've already talked about changing the Bugzilla flags to better express
intent. The BMO gurus say it isn't something we should undertake lightly.
It's much easier to experiment on the commit message / MozReview side of
things while leaving the existing Bugzilla flags in place. e.g. we can have
MozReview convert "land?" in commit messages to "review? flags in Bugzilla
while having autoland convert "land?" into "r=" upon landing.
It's worth noting that part of the reason we haven't enabled feedback flag
support and done more advanced things with commit messages is because
seemingly everyone practices slight variations of interactions with
Bugzilla flags and it's difficult codifying "One True Workflow" because
there are excellent justifications for nearly every variation. Code review
will continue to shift from Bugzilla centric to MozReview centric. And this
means that Bugzilla flags mean less and less over time. Perhaps we can
solve intent in MozReview without having to change anything in Bugzilla...
Personally I'm a little wary of adding too many flags. I think in this
case it should just be left as is. Will there be the occasional
miscommunication and backout as a result? Sure. That's not the end of
the world though.
People who work together often will quickly establish their personal set
of rules and expectations and adjust their workflows accordingly. And
for people who don't work together often, a simple "Please do not land
this yet" is only slightly less convenient than a flag, with the benefit
of being way more explicit.
In the end, it's on the reviewer. If the patch is complicated, has open
dependencies or looks like it might cause problems, as a reviewer I'm
not going to land it no matter what. If it's simple and self-contained,
why not?
-Andrew
On Fri, Jan 22, 2016 at 12:25 AM, Richard Newman <rnew...@mozilla.com>
wrote:
Both of these behaviours are incompatible with reviewer-initiated landing.
I've fallen on both sides of this particular fence; sometimes I want to
fire-and-forget a patch, and sometimes I still want to digest further after
getting review (or I know a piece of work is incomplete and further parts
will be forthcoming).
Perhaps this needs an opt-in flag on the input side?
"r?, and land it if you're happy" versus "r?, but I'll take care of
landing"?
_______________________________________________
firefox-dev mailing list
firefox-...@mozilla.org
https://mail.mozilla.org/listinfo/firefox-dev
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
