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

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.


================
Comment at: clang/www/hacking.html:295-296
+  directory will cause the update of the diff to start a CI run. This dummy
+  file will also add the libc++ group to the list of reviewers. The status of
+  the build will be available in Phabricator.</p>
+
----------------
aaron.ballman wrote:
> I am guessing the dummy file should then be removed before landing the commit and we should document that?
> 
> (FWIW, adding a dummy file feels really unclean as a design -- it's mysterious behavior for new contributors, which the documentation helps with a bit, but also it's a risk for checking in files that are never intended to be in the tree. If there's a way to improve this somehow so we don't need to trick the precommit CI into running, that would be really nice and totally outside of the scope of this review.)
It would be great if just the bootstrapping build could be run on all clang reviews. That should show most problems with changes in clang. If there are any problems in libc++, fixing them would result in a full CI run. Having libc++ run against the patch would probably also be awesome as a smoke test for any clang changes.


================
Comment at: clang/www/hacking.html:311-312
+  <p>Unlike Clang, libc++ supports multiple versions of Clang. Therefore when a
+  patch changes the diagnostics it might be required to use a regex in the
+  "expected" tests to make it pass the CI.</p>
+
----------------
aaron.ballman wrote:
> Should we document the expectation that breaking libc++ due to conforming changes in Clang (in terms of diagnostics and bug fixes, not so much in terms of introducing new language extensions) are generally the responsibility of libc++ maintainers to address, but Clang contributors should attempt to reduce disruptions as much as possible by collaborating with libc++ maintainers when this situation comes up?
That's definitely a good idea. Maybe mention that clang contributors should ping the #libc group to get the attention of libc++ contributors so we can prepare a follow-up patch or, if the author is comfortable with it, also fix libc++ in the same patch (and hopefully get fast approval). Most breakages should be relatively simple to fix.


================
Comment at: libcxx/docs/Contributing.rst:107
+
+The CI tests libc++ for all :ref:`supported platforms <SupportedPlatforms>`.
+The build is started for every diff uploaded. A complete CI run takes
----------------
aaron.ballman wrote:
> philnik wrote:
> > Not really currently. We still claim FreeBSD support without a CI runner.
> Also, do I remember correctly that Windows testing is not done on a CI runner for libc++?
Nope, we have full Windows coverage in the CI. This might have been the case some time before I started working on libc++, but it's definitely not the case anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249