How to submit a change for code review using arc

Project Infrastructure LLVM Dev List Archives
1

I've gone through Code Reviews with Phabriactor[1], Arcanist Quick
Start[2] and Arcanist User Guide arc diff[3]. But I'm unable to setup
reviewers my editor pops up and there is a "Reviewers:" line and but
I'm unable email addresses directly. It seems it wants reviewers
passed on the command line or some how in .arcconfig.

But I haven't been able to find any documentation on how to setup
reviewers so I've guessed you can add it to .git/arc/config:

What I've got now is my .git/arc/config is:
{
  "editor": "vim",
  "reviewers": {
    "peterc": "peter@pcc.me.uk",
    "evgeniys": "eugeni.stepanov@gmail.com",
    "llvm-commits": "llvm-commits@lists.llvm.org"
   }
}

My commit message is:
$ git log -1
commit f04a938e5ad72afbf64242718d456fa480ad5f6b (HEAD ->
compiler-rt-fix-cmake-warnings)
Author: Wink Saville <wink@saville.com>

    [compiler-rt] Fix cmake warnings

    Summary:
    - Fix cmake BOOL misspellings
    - Set cmake policy for CMP0075 to NEW

    Reviewers: peterc, evgeniys

    Subscribers: llvm-commits

The result of "arc diff master" is:
$ arc diff master
You have untracked files in this working copy.

Working copy: /home/wink/prgs/llvm/llvm-project/

Commit message has errors:

      - Error parsing field "Reviewers": The objects you have listed include
      objects which do not exist (peterc, evgeniys).

You must resolve these errors to continue.

    Do you want to edit the message? [Y/n] n

Usage Exception: Message has unresolved errors.

Help appreciated.

[1]: Code Reviews with Phabricator — LLVM 16.0.0git documentation
[2]: ◉ Arcanist Quick Start
[3]: ◉ Arcanist User Guide: arc diff

2

The username has to exist on phabricator:
https://reviews.llvm.org/p/peterc/ -> 404
https://reviews.llvm.org/p/evgeniys/ -> 404

I'm guessing you want:
https://reviews.llvm.org/p/daemon/ -> Evgeniy
https://reviews.llvm.org/p/pcc/ -> Peter Collingbourne

3

Thanks, using daemon and pcc worked. I must say the documentation I
found wasn't helpful, so thanks for the prompt reply!!

4

Hi Wink:

The username has to exist on phabricator:
https://reviews.llvm.org/p/peterc/ → 404
https://reviews.llvm.org/p/evgeniys/ → 404

I’m guessing you want:
https://reviews.llvm.org/p/daemon/ → Evgeniy
https://reviews.llvm.org/p/pcc/ → Peter Collingbourne

Thanks, using daemon and pcc worked. I must say the documentation I
found wasn’t helpful, so thanks for the prompt reply!!

While it’s still fresh in your mind, you might consider updating https://llvm.org/docs/Phabricator.html (which is llvm/docs/Phabricator.rst) based on your experience,
which will make it easier for future contributors. I guess the main issue is getting used to the fact that Phabricator user names are independent and must be looked up on the web site – at least I haven’t found any other way – but that isn’t spelled out anywhere.

thanks again…
don

5

While it's still fresh in your mind, you might consider updating Code Reviews with Phabricator — LLVM 18.0.0git documentation (which is llvm/docs/Phabricator.rst) based on your experience,
which will make it easier for future contributors. I guess the main issue is getting used to the fact that Phabricator user names are independent and must be looked up on the web site -- at least I haven't found any other way -- but that isn't spelled out anywhere.

thanks again...
don

Yes I'll see what I can do to improve the documentation.

Since github seems to be a well known workflow and seems to work OK,
it would be nice if that could be used. Is anyone working on that?

Is there an RFC or some such on what are the requirements for code
reviews/pull requests?

-- Wink

6

While it’s still fresh in your mind, you might consider updating https://llvm.org/docs/Phabricator.html (which is llvm/docs/Phabricator.rst) based on your experience,
which will make it easier for future contributors. I guess the main issue is getting used to the fact that Phabricator user names are independent and must be looked up on the web site – at least I haven’t found any other way – but that isn’t spelled out anywhere.

thanks again…
don

Yes I’ll see what I can do to improve the documentation.

Since github seems to be a well known workflow and seems to work OK,
it would be nice if that could be used. Is anyone working on that?

Is there an RFC or some such on what are the requirements for code
reviews/pull requests?

Not sure if this is what you mean, but here’s a current thread you may find interesting.

https://lists.llvm.org/pipermail/llvm-dev/2019-March/131135.html

hth…
don

7

Yep, that's looks like it, txs.