Closed danielpieczko closed 7 months ago
I reviewed by building the doc and reading the PDF rather than looking at the changes in this PR:
There are a couple of usages of "cmake" that should be "CMake"
I'm wondering if the use of "XCommon CMake" should be just "XCommon" or "XCommon (CMake)"
Sandbox structure - before describing what a sandbox is, first a small like saying "xcommon assumes a sandbox structure" or similar
"There are three types of top-level directories that can be present in a sandbox." We're not defining sandbox's in general here, so something like "there are three types of top-level directories that xcommon supports in a sandbox structure"
Im not sure how well "boilerplate" translates - is there another term we can use?
I think "6.3.4 Best practice" should be "6.3.4 Dependency best practice" or similar
I'm marking this as approved since I feel this PR can be merged as it is, but I feel these items should be addressed
Nice work :)
I've addressed the comments, except the name: I've left XCommon CMake as it is. I was having second thoughts about "boilerplate" as well - I've gone for "CMake Header" in this change.
Fixes #109