[llvm-dev] How to submit a change for code review using arc

237 views
Skip to first unread message

Wink Saville via llvm-dev

unread,
Apr 25, 2019, 11:08:45 PM4/25/19
to llvm-dev
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": "pe...@pcc.me.uk",
"evgeniys": "eugeni....@gmail.com",
"llvm-commits": "llvm-c...@lists.llvm.org"
}
}

My commit message is:
$ git log -1
commit f04a938e5ad72afbf64242718d456fa480ad5f6b (HEAD ->
compiler-rt-fix-cmake-warnings)
Author: Wink Saville <wi...@saville.com>
Date: Tue Apr 23 16:55:21 2019 -0700

[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]: https://llvm.org/docs/Phabricator.html
[2]: https://secure.phabricator.com/book/phabricator/article/arcanist_quick_start/
[3]: https://secure.phabricator.com/book/phabricator/article/arcanist_diff/
_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Doerfert, Johannes via llvm-dev

unread,
Apr 25, 2019, 11:32:16 PM4/25/19
to llvm-dev, Wink Saville
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


---------------------------------------
Johannes Doerfert
Researcher

Argonne National Laboratory
Lemont, IL 60439, USA

jdoe...@anl.gov

________________________________________
From: llvm-dev <llvm-dev...@lists.llvm.org> on behalf of Wink Saville via llvm-dev <llvm...@lists.llvm.org>
Sent: Thursday, April 25, 2019 22:08
To: llvm-dev
Subject: [llvm-dev] How to submit a change for code review using arc

Wink Saville via llvm-dev

unread,
Apr 26, 2019, 1:53:35 PM4/26/19
to Doerfert, Johannes, llvm-dev
On Thu, Apr 25, 2019 at 8:32 PM Doerfert, Johannes <jdoe...@anl.gov> wrote:
>
> 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!!

Don Hinton via llvm-dev

unread,
Apr 27, 2019, 4:43:32 PM4/27/19
to Wink Saville, llvm-dev
Hi Wink:

On Fri, Apr 26, 2019 at 10:53 AM Wink Saville via llvm-dev <llvm...@lists.llvm.org> wrote:
On Thu, Apr 25, 2019 at 8:32 PM Doerfert, Johannes <jdoe...@anl.gov> wrote:
>
> 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

Wink Saville via llvm-dev

unread,
Apr 27, 2019, 8:53:18 PM4/27/19
to Don Hinton, llvm-dev
> 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?

-- Wink

Don Hinton via llvm-dev

unread,
Apr 27, 2019, 10:10:34 PM4/27/19
to Wink Saville, llvm-dev
On Sat, Apr 27, 2019 at 5:53 PM Wink Saville <wi...@saville.com> wrote:
> 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.


hth...
don
 

-- Wink

Wink Saville via llvm-dev

unread,
Apr 28, 2019, 1:59:35 AM4/28/19
to Don Hinton, llvm-dev

Yep, that's looks like it, txs.

Reply all
Reply to author
Forward
0 new messages