[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.


================
Comment at: libcxx/docs/Contributing.rst:88
+* C++XX for the Linux platform (where XX is the latest C++ version).
+* MacOS X86_64 and MacOS arm64 for the Apple platform.
+
----------------
philnik wrote:
> 
They are synonyms, since use the underscore in the CI I prefer to use that name.


================
Comment at: libcxx/docs/Contributing.rst:119
+
+Unless specified otherwise the ``main`` version of Clang is used.
+
----------------
aaron.ballman wrote:
> mstorsjo wrote:
> > Mordante wrote:
> > > mstorsjo wrote:
> > > > I don't understand this paragraph - each CI run is run through the configured set of supported Clang versions - not only `main`? Or does this talk about specifics about manually running tests with the Docker image?
> > > This is the version of Clang in the main branch. How about:
> > > `Unless specified otherwise the nightly build of Clang from the ``main`` branch is used.`
> > I still don't quite understand what this tries to say. "Unless specified otherwise" - where would I specify a different preference, when I upload a patch and it gets run through CI?
> > 
> > There's three different concepts involved here:
> > - The CI build matrix (buildkite-pipeline.yml) which specifies a bunch of different versions of tools and standards to run the tests on. Here you can't specify a different preference - all of them are run (unless you edit the file in your patch).
> > - The Docker images, where the default `clang` executable is a recent nightly build, but older releases are available as `clang-<version>`
> > - The `run-buildbot` script, which just uses whatever compiler is set as default (via e.g. the `CXX` env var).
> > 
> > So, what about these three concepts is this paragraph trying to say - the second or the third point? This really needs to specify what it talks about - build matrix, docker images or run-buildbot script.
> > 
> > The same thing goes for the following paragraph.
> (mostly trying to get rid of the duplicate "Unless specified otherwise".)
I've reworded it, please have a look whether the new wording is clear.


================
Comment at: libcxx/docs/Contributing.rst:142
+
+Builds
+------
----------------
ldionne wrote:
> It feels like this whole section might be prone to becoming out of date. I'm not sure how useful it is since the jobs are pretty self-explanatory. Perhaps it is sufficient to link to `run-buildbot` and `buildkite-pipeline.yml`?
The documentation for these files is already available in this document, more at the end.

I tried to mainly describe the special parts and tried to keep it generic. I could prune a bit more, but I feel some explanation would be good since there are some "suprises" like, formatting allowed to fail, modular build has nothing to do with C++20 modules.


================
Comment at: libcxx/docs/Contributing.rst:231-233
+  export CC=clang-14
+  export CXX=clang++-14
+  run-buildbot generic-cxx17
----------------
philnik wrote:
> Complete nit, but I think `CC=clang-14 CXX=clang++-14 run-builtbot generic-cxx17` as an example would be better to avoid polluting people's environment if they're unfamiliar with a terminal.
Good point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249