[libcxx-commits] [PATCH] D101565: [libcxx] [ci] Add a Windows CI configuration for a statically linked libc++

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius added inline comments.


================
Comment at: libcxx/utils/ci/buildkite-pipeline.yml:321
+  - label: "Windows (DLL)"
+    command: "bash libcxx/utils/ci/run-buildbot generic-win"
+    artifact_paths:
----------------
?


================
Comment at: libcxx/utils/ci/buildkite-pipeline.yml:332
+  - label: "Windows (Static)"
     command: "bash libcxx/utils/ci/run-buildbot generic-win"
     artifact_paths:
----------------
Didn't you intend to do it this way?


================
Comment at: libcxx/utils/ci/run-buildbot:494
+    CMAKE_FLAGS=""
+    if [ "${BUILDER}" = "generic-win-static" ]; then
+        CMAKE_FLAGS="$CMAKE_FLAGS -DLIBCXX_ENABLE_SHARED=NO"
----------------
I'm not a big fan of having this if inside case. I'd rather go for factoring out common flags to a function.
The current way is harder to reason about IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101565