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

Andrzej Warzynski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 16 03:41:36 PST 2021


awarzynski added inline comments.


================
Comment at: llvm/docs/Phabricator.rst:187-188
+that you have a series of commits that you have not posted for review and use
+`arc` to upload them. They can be adapted if you're using the web interface,
+or updating existing reviews.
+
----------------
jhenderson wrote:
> awarzynski wrote:
> > I would remove this last sentence. In this case, IMHO, less is more.
> > 
> > Basically, this is a bit complex and I think that focusing on `arc diff` + `git rebase` is sufficient. Yes, you can use the web UI too, but perhaps that's something that people can deduct here themselves?
> I wouldn't agree here (as someone who doesn't use `arc` at all, if I read this paragraph without the sentence, I'd just skip the rest of the example, as it "doesn't apply to me"), but on reread, this example really isn't all that applicable to the Web UI, since commit messages aren't directly linked to patch descriptions (i.e. modifying the local commit message has no impact on anything in the web UI).
Fair point. Maybe the first sentence should be rephrased to be a bit more general:
```
One frequently used workflow for creating a series of patches involves git's rebasing. These steps assume that you have a series of commits that you have not posted for review.
```

Then, replace: 
```
- ``arc diff`` to upload for review.`
```
with:
```
- upload the current commit for a review (either with ``arc diff`` or via the we UI)
```

This way it should work for everyone (other steps are already general enough).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115519



More information about the llvm-commits mailing list