I’m working with Manuel Klimek and Sam McCall to streamline the code review process for LLVM/Clang/etc. diffs.
tl;dr: I want to set up separate Differential repositories for each LLVM/Clang/etc. project, so Herald can automatically Cc: the correct list for each review request.
We don’t want to unnecessarily Cc: this list for non-Clang review requests, so we need a solution to set up the proper Cc:s for each review.
Currently, the LLVM project uses a single Differential repository for all projects, including clang, clang-tools-extra, etc. — but we don’t want to Cc: cfe-commits on review requsts for LLVM so we cannot match on repository.
If we used a monorepo, we could match on path to assign the Cc:, but we have separate .arcconfig files already at the root of each repository.
This means Differential gets project-relative paths in its review requests, which means we cannot use Herald to easily distinguish diffs meant for different projects.
For example, if a review request is for lib/CMakeLists.txt, we have no way to know if that is for llvm/lib/CMakeLists.txt or llvm/tools/clang/lib/CMakeLists.txt, etc.
Since we already have .arcconfig files at the base of each repository, we can just create separate Differential repositories corresponding to each .arcconfig, then set up Herald rules to automatically Cc: the appropriate list for each review request.
As noted in the tl;dr: above, I set up a proof-of-concept repository for clang-tools-extra in https://reviews.llvm.org/D40179 and a Herald rule in https://reviews.llvm.org/H268 to confirm this technique works.
Separate Differential repositories require a bit more back-end storage and CPU on the Phabricator instance (but aside from LLVM and Clang, I think the other repos are small enough that this won’t be significant.)
Moving forward, only LLVM commits will be identified with the prefix rL (as in https://reviews.llvm.org/rL12345) — each project will get its own unique prefix, which might surprise some users or require changes to tools which bake in the rL prefix
Moving to a monorepo in the future won’t require too much work, but we will either need to move to a single Differential repository and re-adjust the Herald rules to handle absolute paths, or continue using separate Differential repositories even though the underlying storage is a monorepo.
Please share thoughts and feedback, and thanks!