[cfe-dev] [Proposal] Automatically Cc: cfe-commits@ on Clang reviews

[Ben Hamilton via cfe-dev]

Forgot to mention:

> Also, how would arcanist pick up two callsigns here? Wouldn't it just
pick the from the closest surrounding .arcconfig?

The review request will belong to a single repository, as you noticed (from
the closest .arcconfig to where the arc command was invoked).

However, when commits land, they will be imported under either one (rL for
LLVM diffs) or two (rL + rC for e.g. Clang diffs) repositories.

Ben

On Mon, Nov 27, 2017 at 10:00 AM Ben Hamilton <benhamilton at google.com>
wrote:

> Good point, and sorry for the confusion.
>
> Following the monorepo setup instructions
> <https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo>,
> diffs sent out from a monorepo will use llvm/.arcconfig
> <https://reviews.llvm.org/diffusion/L/browse/llvm/trunk/.arcconfig>.
>
> I haven't committed any changes to that .arcconfig yet, and in addition
> there aren't yet any Herald rules which copy llvm-commits@ on review
> requests sent out to the LLVM project.
>
> So, that explains why llvm-commits@ was not copied on D40312.
>
> The previous setup made sense, since review requests for clang et al were
> also lumped together in the rL LLVM Diffusion repository
> <https://reviews.llvm.org/diffusion/L/>, and we didn't want to subscribe
> llvm-commits@ on those.
>
> Once I land the set of commits so each project has its own Diffusion
> repository (and we give it a week or so so everyone is up to date), we can
> update llvm/.arcconfig to explicitly set revisions as belonging to the rL
> repository, and then set up a Herald rule to subscribe llvm-commits@ on
> those.
>
> Does that help?
>
> Ben
>
> On Mon, Nov 27, 2017 at 9:42 AM Philip Pfaffe <philip.pfaffe at gmail.com>
> wrote:
>
>> Hi Ben,
>> (+llvm-dev since the email I'm replying to wasn't sent there)
>>
>> 2017-11-21 17:18 GMT+01:00 Ben Hamilton via cfe-dev <
>> cfe-dev at lists.llvm.org>:
>>
>>> OK. I confirmed that Stephan's process to send out cross-repo diffs from
>>> the monorepo is not affected by my proposal.
>>>
>>> I'm a bit late to the party, and I might just not have comprehended this
>> correctly. But does your example actually work? I didn't see any email
>> about D40312 on llvm-commits, shouldn't there have been one? Also, how
>> would arcanist pick up two callsigns here? Wouldn't it just pick the from
>> the closest surrounding .arcconfig?
>>
>>
>>
>> tl;dr: https://reviews.llvm.org/D40312 (which was sent out on top of
>>> D40179 to emulate what happens with a monorepo after my proposal lands).
>>>
>>> Steps I took (starting with
>>> https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo
>>> ):
>>>
>>> ===
>>> % git clone https://github.com/llvm-project/llvm-project-20170507/
>>> llvm-project
>>> % cd llvm-project
>>> % cp llvm/.arcconfig .
>>> % mkdir -p .git/info
>>> % echo .arcconfig >> .git/info/exclude
>>> # Manually apply D40179 into the clang-tools-extra directory
>>> % arc export --revision D40179 --git | patch -d clang-tools-extra -p1
>>> patching file .arcconfig
>>> % git diff
>>> diff --git a/clang-tools-extra/.arcconfig b/clang-tools-extra/.arcconfig
>>> index f846581763e..d4a00161bce 100644
>>> --- a/clang-tools-extra/.arcconfig
>>> +++ b/clang-tools-extra/.arcconfig
>>> @@ -1,4 +1,4 @@
>>>  {
>>> -  "project_id" : "clang-tools-extra",
>>> +  "repository.callsign" : "CTE",
>>>    "conduit_uri" : "https://reviews.llvm.org/"
>>>  }
>>>  % git commit -a -m "Differential Revision:
>>> https://reviews.llvm.org/D40179"
>>>  % echo "Test 1" >> llvm/README.txt
>>>
>>>  % echo "Test 2" >> clang-tools-extra/README.txt
>>>
>>>  % git commit -a -m "[Test] Monorepo test diff crossing Differential
>>> repositories"
>>>  % arc diff --create --reviewers sberg HEAD^
>>> ===
>>>
>>> Ben
>>>
>>> On Tue, Nov 21, 2017 at 8:00 AM Stephan Bergmann <sbergman at redhat.com>
>>> wrote:
>>>
>>>> On 11/21/2017 03:41 PM, Ben Hamilton wrote:
>>>> >  > My understanding was that, while there currently is no official
>>>> > monorepo, it is at least possible and accepted to send single,
>>>> > cross-repo patches for review.  That would no longer be possible,
>>>> right?
>>>> >
>>>> > I assume cross-repo patches would be constructed by hand, right? Or do
>>>> > you mean using the unofficial monorepo from 2016
>>>> > <https://github.com/joker-eph/llvm-project>?
>>>>
>>>> I'm using <https://github.com/llvm-project/llvm-project-20170507> (as
>>>> advertised at
>>>> <
>>>> https://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo
>>>> >),
>>>> where such a "cross-repo diff" is just a matter of a plain 'git diff'.
>>>>
>>>> > An example of an existing cross-repo review in Differential would be
>>>> > nice—do you happen to have one handy?
>>>>
>>>> I just happened to file <https://reviews.llvm.org/D40295> a moment ago,
>>>> spanning clang and compiler-rt.
>>>>
>>>> > I think either way, we'd keep the ability to send cross-repo patches
>>>> to
>>>> > the top-level LLVM repo just the same way you could today.
>>>> >
>>>> > I'll confirm that if you can help me construct an example cross-repo
>>>> > patch for review.
>>>>
>>>> Thanks,
>>>> Stephan
>>>>
>>>
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20171127/fb62e096/attachment.html>