<div dir="ltr">If I understand your question correctly, this is not quite correct.<br><br>The basic unit of operation in Phabricator is the "Revision" which<br>roughly corresponds to thing to be reviewed. Within Revisions, there<br>are Diffs, which correspond to different specific concrete versions of<br>Revisions. So, if you for instance had a change that corresponded to a<br>single reviewable unit, you would submit that as a Revision using "arc<br>diff". *Neither of these meaningfully corresponds to revision control<br>commits*.<br><br>So, say for the sake of argument you are working for some feature, and<br>the git history consists of:<br><br> origin/master -> A1 -> A2 -> A3<br><br>When you do |arc diff|, this will create a single Revision (say D1)<br>which contains all of A1, A2, A3. If you have to make some changes in<br>response to review and you now have:<br><br> origin/master -> A1 -> A2 -> A3 -> A4<br><br>When you do |arc diff|, this will update Revision D1 to consist<br>of the changes in A1, A2, A3, A4. <br><br><br>Say you instead have:<br><br> origin/master -> A1 -> A2 -> A3 -> B1 -> B2<br><br>Where A and B correspond to different reviewable units, you would<br>make two Revisions, one consisting of A1, A2, A3 and one consisting<br>of B1, B2 (operationally, this is done by doing:<br><br> with HEAD = A3: arc diff // Creates D2<br> with HEAD = B2: arc diff A3 // Creates D3 and bases it off A3<br><br>And you would mark these revisions in a parent child relationship so<br>that D2 is the parent of D3. Obviously, this is scriptable, though I<br>don't think arc diff has a script for it.<br><br>So, if, as before, you want to revise D2 (corresponding to the A<br>series of commits) [0], then you would end up with history:<br><br> origin/master -> A1 -> A2 -> A3 -> A4 -> B1 -> B2<br> ^<br><div> New</div><div><br></div><div>Note that you don't amend commits in this model (though of course</div><div>you could). You just add another commit to the end of the commits</div><div>in the Revision.<br></div><div><br></div>And you would then do:<br><br> with HEAD = A4: arc diff<br><br>This will automatically update D2 while leaving it in the same<br>parent/child relationship.<br><br>-Ekr<br><br>[0] Note that I'm assuming you want the response to the review<br>to be attached to A as opposed to slapped on the end of the<br>branch as in Github.<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 6, 2018 at 5:57 PM, Chris Pearce <span dir="ltr"><<a href="mailto:cpearce@mozilla.com" target="_blank">cpearce@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Reading through <a href="https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#series-of-commits" target="_blank">https://moz-conduit.<wbr>readthedocs.io/en/latest/<wbr>phabricator-user.html#series-<wbr>of-commits</a> it seems the actions that you have to do to push a review request with a commit series is to run `arc diff` on each commit in the series, and then load up the Phabricator UI and manually associate the parent-child relationships between all commits? So if you then amend the bottom most commit and then want to resubmit, you need to repush the entire series and then re-set the parent/child relationships? That sounds rather manual and error prone, and a significant UX regression from simply typing `hg push review`.</div><div><br></div><div>The review process is critical to how we ship code, so I don't think we should accept making requesting and performing reviews harder and more error prone.</div><div><br></div><div>If the problem is that it was decided we are moving out of a datacenter, can we write a cheque to extend our lease there, or move the Phabricator machine (or an image of it) somewhere else?</div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 7, 2018 at 12:18 PM, Chris Pearce <span dir="ltr"><<a href="mailto:cpearce@mozilla.com" target="_blank">cpearce@mozilla.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>A quick `hg log` on mozilla-central shows me that the commit series workflow is extremely common. I use it myself a lot, and believe it is good practise. So I would be very disappointed if we made it inconvenient to break up work in this way.</div><div><br></div><div>I'm not trying to add stop energy to improving our tools and processes, but I feel taking a review UX regression in the short term is not a good trade off. Can we please delay obsoleting MozReview until Phabricator has better support for the commit-series workflow?<br></div><div><br></div><div><br></div><div>Regards,</div><div>Chris Pearce.<br></div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-2501473621670361474h5">On Thu, Jun 7, 2018 at 9:44 AM, Jean-Yves Avenard <span dir="ltr"><<a href="mailto:jyavenard@mozilla.com" target="_blank">jyavenard@mozilla.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="m_-2501473621670361474h5"><span><br>
<br>
> On 6 Jun 2018, at 7:54 pm, Markus Stange <<a href="mailto:mstange@themasta.com" target="_blank">mstange@themasta.com</a>> wrote:<br>
> <br>
> Is there a list of features which are planned to be implemented in Phabricator + Lando before MozReview is sunset? More specifically, is Phabricator expected to have first class support of the "commit series" development workflow by the time this happens?<br>
> This is a feature that I personally would miss a lot. Of the 520 patches I've landed within the last two years, only 132 patches were from single-patch bugs. 262 patches landed as part of a 5-commits-or-more series.<br>
> <br>
> Thanks,<br>
> Markus<br>
<br>
</span>Wait what?<br>
<br>
No ability to submit a series of patch ???<br>
<br>
I can’t recall the last time I pushed a single patch to mozreview. 1 commit = 1 scope, that’s the way it should always be.<br>
<br>
I don’t understand how we could consider a compulsory switch to phabricator, or even discuss a switch to it without support of this essential feature.<br>
<span class="m_-2501473621670361474m_-7915872636847350786HOEnZb"><font color="#888888"><br>
JY<br>
<br>
<br>
</font></span><br></div></div><span>______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listi<wbr>nfo/firefox-dev</a><br>
<br></span></blockquote></div><br></div>
</blockquote></div><br></div>
</div></div><br>______________________________<wbr>_________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/<wbr>listinfo/firefox-dev</a><br>
<br></blockquote></div><br></div>