[PATCH] D136392: [Bazel] Use `LLVM_VERSION` from `llvm/CMakeLists.txt`

NAKAMURA Takumi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 12 06:47:26 PST 2023


chapuni added inline comments.


================
Comment at: utils/bazel/configure.bzl:143
 
+    vars = _extract_cmake_settings(
+        repository_ctx,
----------------
rupprecht wrote:
> I see a warning that `vars` is unused here and should be removed. Did you mean to have `_extract_cmake_settings()` only parse the information, and have a separate helper method that actually writes out the file, i.e. split the previous method at `# Write out individual vars`?
I just thought `vars` might be used later after `_extract_cmake_settings` in  `_llvm_configure_impl`.

> Did you mean to have _extract_cmake_settings() only parse the information, and have a separate helper method that actually writes out the file, i.e. split the previous method at # Write out individual vars?

I could just remove an assignment but it really makes sense.
I have introduced `_write_dict_to_file`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136392



More information about the llvm-commits mailing list