<table><tr><td style="">rkflx added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D10797">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D10797#241886" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D10797#241886</a>, <a href="https://phabricator.kde.org/p/simgunz/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@simgunz</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>My doubt is: when you want to review a "review" what do you do exactly (I never did it). You download a patch and apply it manually to your code (probably arc does this with some command)?</p></div>
</blockquote>
<p>Anyone wanting to review or simply try out a patch usually just issues <tt style="background: #ebebeb; font-size: 13px;">arc patch D10797</tt>. This will make sure the repo is up-to-date to be able to find the commit you attached your revision to, create a new local branch named <tt style="background: #ebebeb; font-size: 13px;">arcpatch-D10797</tt>, download all dependent revisions and finally apply the patch. In case the author updates the patch, the reviewer will re-issue <tt style="background: #ebebeb; font-size: 13px;">arc patch</tt>, get a new branch called <tt style="background: #ebebeb; font-size: 13px;">arcpatch-D10797_1</tt> and so on. Note that on these branches there is only a single squashed commit per Diff, i.e. (at least currently, might change in the future) what you see in <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Revision Contents</span></span><span style="color: #92969D;"> → </span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Commits</span></span></span> is only visible on Phab.</p>
<p>To investigate what the author changed, you could compare both local branches, but I guess most people just just the options for comparing revisions under <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Revision Contents</span></span><span style="color: #92969D;"> → </span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">History</span></span></span>.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>If I overwrite some commits, as I did today, does it create any problem to the reviewer that already "pulled" from arc the review once? (in git this will create diverging branches between local and remote) Or does arc redownload the diff completely each time, so avoiding problems?</p></blockquote>
<p>Phabricator remembers all Diffs you uploaded. <tt style="background: #ebebeb; font-size: 13px;">arc patch</tt> will simply re-download the complete patch, using the latest revision unless you specify an older revision.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>This was not my fear, but I feared to mess up things to the reviewer that already "pulled" the review before.</p></blockquote>
<p>Nothing to fear. If it looks good in the Diff viewer on Phabricator, it will look good for your reviewer.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>It was complaining about the fact that the commit with the revert did not contain a valid "Differential Revision" ID</p></blockquote>
<p>Ah, my comment might have been misleading. This line will only get added once you do <tt style="background: #ebebeb; font-size: 13px;">arc patch</tt> or <tt style="background: #ebebeb; font-size: 13px;">arc amend</tt>. However, you won't need to issue those commands or even add the line manually, because Arcanist will do it for you once you <tt style="background: #ebebeb; font-size: 13px;">arc land</tt>.</p>
<p>There is a second way <tt style="background: #ebebeb; font-size: 13px;">arc diff</tt> can recognize the revision, which works by comparing your local commit sha with what has been recorded on Phabricator (IIRC – if not, <tt style="background: #ebebeb; font-size: 13px;">arc which</tt> will tell you the real reason). If you change things in such a way that the commit sha on your local branch changed (which should not happen if you just add a reverting commit), then you'd have to manually provide the revision ID.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>I'll also have a look at <a href="https://phabricator.kde.org/T7116" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">T7116</a> and maybe I can give some feedback from a point of view of a newcomer.</p></blockquote>
<p>That would be appreciated ;) (Or just update the Wiki yourself.)</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D10797">https://phabricator.kde.org/D10797</a></div></div><br /><div><strong>To: </strong>simgunz, Okular, aacid<br /><strong>Cc: </strong>rkflx, aacid, ngraham, michaelweghorn<br /></div>