doc: Improve cmake README and INSTALL docs #1448
Conversation
cmake/README.md
Outdated
|
|
||
| ```bash | ||
| mkdir /tmp/my-build | ||
| cd /tmp/my-build | ||
| ``` | ||
|
|
||
| Or you may also create a build directory within the oneTBB source directory. |
There was a problem hiding this comment.
Purpose of this addition is not clear. A temporary build directory can be created in an arbitrary location - this is what is stated in the original message above.
Probably it is worth to add a better example instead of changing the general instructions.
There was a problem hiding this comment.
Yea. What do you think about changing the examples here to be out of source build?
There was a problem hiding this comment.
Also, the install doc refers .. location multiple times as the repo directory, which implies the build directory is a sub-directory of the repo directory. This might need to be updated as well.
ChihweiLHBird
force-pushed
the
zhiwei/improve-cmake-readme
branch
2 times, most recently
from
b63e220 to
1ad310a
Compare
|
I just updated it to address some concerns, let me know if you have other suggestions. Thanks! |
ChihweiLHBird
changed the title
cmake/README.md
Outdated
| @@ -35,8 +35,10 @@ cd /tmp/my-build | |||
|
|
|||
| ### Configure | |||
|
|
|||
| In the build directory, specify the oneTBB source directory to generate a build system for it. | |||
There was a problem hiding this comment.
I don't personally think that this comment adds useful information here. Flow of this README already implies that the user's current working directory to be the build directory after the steps from "Preparation" section.
There was a problem hiding this comment.
It might be the case for users who are familiar with cmake, but for users who don't, it can be very confusing because they have no idea why running an unfamiliar command in an empty directory and what does it do.
In many open-source community projects, the assumption of docs usually is that users are complete beginners for those tech stack and tooling.
There was a problem hiding this comment.
But if you guys strongly against it, feel free to suggest a change and I will follow.
There was a problem hiding this comment.
https://github.com/oneapi-src/oneTBB/blob/master/cmake/README.md#configure-build-and-test is a step-by-step guide to configure, build and test oneTBB. So the document is assuming that the reader follows it from top to bottom.
I think you could probably replace <repo_root> by <onetbb_repo_directory> and add From the build directory, run: as it has been done for INSTALL.md. Will it help?
There was a problem hiding this comment.
Thanks for the idea! How does it look now?
There was a problem hiding this comment.
And yes, it's fine to assume the developer is reading from top to bottom; however, in my opinion, it's also very import to descript what each step does in a README doc rather than just put a bunch of commands for the developer to run without explanations, especially for cmake and C++ which has a very different build workflow than other common programming languages (e.g., Java, Python, Go).
cmake/README.md
Outdated
| @@ -35,8 +35,10 @@ cd /tmp/my-build | |||
|
|
|||
| ### Configure | |||
|
|
|||
| In the build directory, specify the oneTBB source directory to generate a build system for it. | |||
Co-authored-by: Ilya Isaev <[email protected]>
Co-authored-by: Alexandra <[email protected]>
…ation description
ChihweiLHBird
force-pushed
the
zhiwei/improve-cmake-readme
branch
from
39ba3a8 to
39597d9
Compare
Description
Making the README more clear for beginner C++ developers (like me).
Type of change
Choose one or multiple, leave empty if none of the other choices apply
Add a respective label(s) to PR if you have permissions
Tests
Documentation
Breaks backward compatibility
Notify the following users
List users with
@to send notificationsOther information