Feedback

[arm...@mozilla.com]

Hello,
I've been using Mozreview for a while and I'm happy with the changes I'm seeing. Speciall the UI refresh. Good job!

One of the issue I keep on hitting is when a part of my patch has already landed and I want to share more code to be reviewed. [1]

I also hit the following issue [2]

• I get someone to review my patch
• I get r+ plus some nites
• I fix the nits, I amend and I push back to mozreview
• I tried to use autoland to land it for me
• I can't because I did not trigger try jobs through mozreview
• I decide to rebase my bookmark locally
• hg tells me that the changeset is already public

Do we need to enforce that a try request was done through mozreview to autoland?
Am I expected to do all my try experimenting through mozreview?

Tbh, I find it very difficult to be expected to do my *multiple* try runs through mozreview.

The flow with "./mach try <syntax>" is so well done that I can't see myself using mozreview for my try pushes.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1239320
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1244733

[arm...@mozilla.com]

I also forgot to ask, is there a way to push to mozreview with a flag saying "yes, autopublish without being prompted"

I sometimes push and go on doing other things and I *completely* forget that I have to wait until I can answer the question.
The question takes too long until it appears.

[Gregory Szorc]

On Mon, Feb 1, 2016 at 7:12 AM, <arm...@mozilla.com> wrote:

Hello,
I've been using Mozreview for a while and I'm happy with the changes I'm seeing. Speciall the UI refresh. Good job!

One of the issue I keep on hitting is when a part of my patch has already landed and I want to share more code to be reviewed. [1]

I also hit the following issue [2]

• I get someone to review my patch
• I get r+ plus some nites
• I fix the nits, I amend and I push back to mozreview
• I tried to use autoland to land it for me
• I can't because I did not trigger try jobs through mozreview
• I decide to rebase my bookmark locally
• hg tells me that the changeset is already public

You shouldn't see this error on rebase just from pushing to MozReview. Either you pushed your changesets to a "publishing" repo (including if you pulled them from 2 repos locally since repos are publishing by default) or you are trying to rebase the wrong changesets. If you leave out the -s argument from `hg rebase`, it will attempt to rebase everything from the common ancestor. If e.g. your changes are based on top of inbound and you try to rebase onto central, it will attempt to rebase inbound onto central and this will give an error about public changesets not being mutable. The workaround here is to use `hg wip` and `hg rebase -s <start> -d <dest>` to explicitly specify the source revision to start the rebase from.

 

Do we need to enforce that a try request was done through mozreview to autoland?

There is active discussion on this topic and a handful of bugs have been filed. We want to eventually settle on "yes" so we can remove the integration repositories. Although, in that world Autoland may trigger the Try run automatically if it hasn't run already.

 

Am I expected to do all my try experimenting through mozreview?

You can still push to Try without MozReview. You are not expected to do all experimenting through MozReview. Although over time there will likely be more compelling reasons to experiment through MozReview.

 

Tbh, I find it very difficult to be expected to do my *multiple* try runs through mozreview.

The flow with "./mach try <syntax>" is so well done that I can't see myself using mozreview for my try pushes.

The UI for triggering Try from MozReview is still a bit rough. It originally started as the obvious first feature of Autoland to implement, not a "push to try" replacement.

 

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1239320
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1244733

--
You received this message because you are subscribed to the Google Groups "mozilla-code-review" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mozilla-code-re...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Gregory Szorc]

On Mon, Feb 1, 2016 at 7:15 AM, <arm...@mozilla.com> wrote:

I also forgot to ask, is there a way to push to mozreview with a flag saying "yes, autopublish without being prompted"

[reviewboard]

autopublish = true

 

 

I sometimes push and go on doing other things and I *completely* forget that I have to wait until I can answer the question.
The question takes too long until it appears.

The latency bothers me too. I recently made changes that should have drastically reduced it. But things are still slow. I have a patch under review adding more logging so we can hopefully pinpoint the slowdown so we can make it faster.

 

On Monday, 1 February 2016 10:12:31 UTC-5, arm...@mozilla.com wrote:

Hello,
I've been using Mozreview for a while and I'm happy with the changes I'm seeing. Speciall the UI refresh. Good job!

One of the issue I keep on hitting is when a part of my patch has already landed and I want to share more code to be reviewed. [1]

I also hit the following issue [2]

• I get someone to review my patch
• I get r+ plus some nites
• I fix the nits, I amend and I push back to mozreview
• I tried to use autoland to land it for me
• I can't because I did not trigger try jobs through mozreview
• I decide to rebase my bookmark locally
• hg tells me that the changeset is already public

Do we need to enforce that a try request was done through mozreview to autoland?
Am I expected to do all my try experimenting through mozreview?

Tbh, I find it very difficult to be expected to do my *multiple* try runs through mozreview.

The flow with "./mach try <syntax>" is so well done that I can't see myself using mozreview for my try pushes.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1239320
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1244733

[Armen Zambrano Gasparnian]

Thank you gps! Specially the explanation about rebasing. I was doing it wrong!

I thought we were enforcing a "try push" because autoland to mi was disabled when I tried.

--

Zambrano Gasparnian, Armen
Automation & Tools Engineer
http://armenzg.blogspot.ca