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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for the review comments. I'll address them in a little while. I want to wait for more review comments.



================
Comment at: clang/www/hacking.html:302
+
+  <p>For most builds, the pre-commit CI uses a recent
+  <a href="https://apt.llvm.org/">nightly build</a> of Clang from LLVM's main
----------------
mstorsjo wrote:
> I'm wondering if there's a better synonym for "builds" here which would be clearer? Initially I read this as if "most times a patch is run through the CI", not "most of the configurations in the CI run". So maybe "configurations"?
Agreed configurations seems more appropriate in this context.


================
Comment at: libcxx/docs/Contributing.rst:119
+
+Unless specified otherwise the ``main`` version of Clang is used.
+
----------------
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.`


================
Comment at: libcxx/docs/Contributing.rst:158
+* ``Clang XX`` these build steps test whether the changes work with all
+  supported Clang versions.
+* ``Booststrapping build`` builds Clang using the revision of the patch and
----------------
mstorsjo wrote:
> Side note - these tests only test one specific version of C++ with these versions of Clang - we don't have fully coverage of all standard versions with all supported versions of Clang. (Not really sure if that's worth mentioning though. Also I do see this kinda hinted at at the end of the file.)
We might spell it our more explicitly. But maybe not here but where the hints are. There are a lot of possible permutations not tested. For example the modular build is only tested with C++2b.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249