[PATCH] D105822: [docs] More CMAKE variable documentation

[Nathan Sidwell via Phabricator via llvm-commits]

urnathan marked 3 inline comments as done.
urnathan added inline comments.


================
Comment at: llvm/docs/CMake.rst:224
+
+Frequently Used LLVM-specific variables
+-----------------------
----------------
aaron.ballman wrote:
> I'd move this up above the rarely-used section so that the list goes in order of most-used to least-used. WDYT?
I wasn;t sure myself, your comment is enough to do the ordering you suggest.


================
Comment at: llvm/docs/CMake.rst:225
+Frequently Used LLVM-specific variables
+-----------------------
+
----------------
aaron.ballman wrote:
> Underlines are off here, so sphinx will complain about this.
I don't know what that means -- I tried to figure out how to sphinx it, but failed.  I (attempted to) match the formatting of other section headers.


================
Comment at: llvm/docs/CMake.rst:228
+The default configuration may not match your requirements. Here are
+LLVM variables that are frequently used to control that. The full
+description is in `LLVM-specific variables`_ below.
----------------
aaron.ballman wrote:
> One thing that's interesting about this is that I don't know that they're LLVM variables or not. `LLVM_ENABLE_PROJECTS` isn't really LLVM-specific, it decides what to build as part of the whole project. Similar logic applies to a lot of these items. The only one that's really LLVM-specific in this list is `LLVM_TARGETS_TO_BUILD`, but even that one impacts Clang.
Agreed, there's an unfortunate use of LLVM to mean both 'LLVM library' and 'LLVM project'.  I don't know the best way to disambiguate that -- or how to categorize these variables.


================
Comment at: llvm/docs/CMake.rst:261
+These variables provide fine control over the build of LLVM and
+enabled sub-projects.  Nearly all of these variable namess begin with
+``LLVM_``.
----------------
aaron.ballman wrote:
> 
muscle memory for the lose!


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

https://reviews.llvm.org/D105822