[PATCH] D115519: [llvm][docs] Describe how to work with patch series on Phabricator

[David Spickett via Phabricator via llvm-commits]

DavidSpickett marked 5 inline comments as done.
DavidSpickett added inline comments.


================
Comment at: llvm/docs/Phabricator.rst:156
+"Edit Child Revisions" option instead.)
+
+The second method is less work for you but assumes you are using `arc`. This can be
----------------
DavidSpickett wrote:
> Probably going to add sub-titles for each method "with web interface", "with arc" just don't have the time to get to it today.
On second thought there's not too much text here that it needs it.


================
Comment at: llvm/docs/Phabricator.rst:157-161
+The second method is less work for you but assumes you are using `arc`. This can be
+done when updating a review as well as uploading it for the first time.
+
+* Upload the first review with `arc`. Note its patch number.
+* For each commit after that add a line to the commit message. "Depends on D<num>"
----------------
awarzynski wrote:
> jhenderson wrote:
> > I don't believe you need to use `arc` for this process. I've used the "Depends on" trick many times, and I don't use `arc`. You can do it in the patch description in the Web UI too.
> > 
> > Possible alternative phrasing in the inline edit, including a couple of other suggested tweaks ("is less" is subjective, whereas "may be less" will always be true (in that "not being less" is covered under "may").
> > I don't believe you need to use arc for this process.
> 
> That's true. In fact, there are at least 3 methods:
> # exclusively with the web UI
> # exclusively with `arc`
> # web UI mixed with `arc`
> 
> Originally David only documented 1. and 2. For me that's sufficient and it makes a lot of sense - there are 2 paragraphs and 2 "completely" different methods. IMO, adding 3. to the mix blurs the message a bit ("essentially, there are 2 ways to achieve this"). Perhaps "less is more" in this case?
On reflection I think the 2 routes are "Depends" with arc or web, and "Edit parent" with only the web. So I've rephrased it that way.


================
Comment at: llvm/docs/Phabricator.rst:164
+  This must be entirely on its own line, with a blank line before it.
+  For example::
+
----------------
jhenderson wrote:
> Should this really be a double colon?
This makes the following text into a box.
{F21041853}


================
Comment at: llvm/docs/Phabricator.rst:166-170
+    [llvm] Example commit for Phabricator docs
+
+    Differential revision: https://reviews.llvm.org/D12345
+
+    Depends on D12344
----------------
jhenderson wrote:
> I think you want to put this in some kind of no-format/code markdown or similar.
Switched to plain text boxes or double backtick for all the commands.


================
Comment at: llvm/docs/Phabricator.rst:168
+
+    Differential revision: https://reviews.llvm.org/D12345
+
----------------
awarzynski wrote:
> This line will be added by Arcanist _after_ you run `arc diff`, right?
Removed that line.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115519/new/

https://reviews.llvm.org/D115519