1311968 - Support landing revisions in the Draft state to try

Status: NEW

(Conduit :: Lando, defect, P3)

Description

[Mike Hommey [:glandium]]

The current shortest workflow with mozreview, involving a push to try, looks like:
- Create a local commit with r?person syntax
- Push to mozreview
- Publish the draft
- Autoland to try
- Reviewer r+es
- Someone pushes the button to land on the autoland branch.

The problem with the above is that you can only autoland to try after the draft is published, while the push data is in the hg-reviewboard repository, and could very well be pushed to try.

What this means is that there is actually a time frame where someone is flagged r? but the requester may actually prefer to wait for try results.

It would be nice if they could do that without having to push to try on their own. This could be considered as "hg-reviewboard is the new try".

Comment 1

[Gregory Szorc [:gps]]

One of the things I'm working on this quarter will be an API on Treeherder that answers "is this push green." I don't think we'll be able to do it by the end of the quarter (because auto starring isn't perfect yet), but one could imagine the behavior of MozReview is to automatically push to try then only take actions if the try push is green. e.g. we could hold off publishing the draft until green try run. Or we could hold off requesting review until a green try run. Or we could cancel review if the try run was bad.

FWIW, the reason we have to publish before pushing to try is because Autoland calls Review Board's APIs to post results. And the Review Board ACLs tend to only allow the submitter to update draft review requests. We could make an exception for autoland if we really wanted, I think.

Comment 3

[Mark Côté [:mcote]]

This is something I'm considering for Phabricator-Lando.

Comment 4

[Steven MacLeod [:smacleod]]

This bug is pretty specific to MozReview's permissions model and how drafts work. The same thing isn't an issue with Phabricator, so I think a WONTFIX here makes more sense.

Comment 5

[Mark Côté [:mcote]]

We still want a mechanism to submit to Try via Phabricator, and using a draft mode there makes a lot of sense to me.  Based on a previous conversation, I think "Phabricator-Lando is the new try" is an eventual goal here.  And comment 1 is still relevant (although I'd call that a future enhancement).

Rather than file a new bug and see-also this one, we can just put in a user story with the differences in the Phabricator-Lando world.

Comment 6

[Rob Wu [:robwu]]

In Phabricator one can create a diff without a review via "arc diff --preview": https://secure.phabricator.com/book/phabricator/article/arcanist_diff/#force-diff-only

Do you want to support landing these diffs to try?
If not, then this bug should probably be closed in favor of bug 1466275.
If yes, then this bug should probably depend on (or "see also" to) bug 1466275.

Comment 7

[Steven MacLeod [:smacleod]]

(In reply to Rob Wu [:robwu] from comment #6)
> In Phabricator one can create a diff without a review via "arc diff
> --preview":
> https://secure.phabricator.com/book/phabricator/article/arcanist_diff/#force-
> diff-only
> 
> Do you want to support landing these diffs to try?
> If not, then this bug should probably be closed in favor of bug 1466275.
> If yes, then this bug should probably depend on (or "see also" to) bug
> 1466275.

I don't think we want to support sending revision-less diffs to try, but revisions now have a "Draft" state where they don't request review (and can be blocked on various things). I'll leave this open to track being able to send those to try.