-
Notifications
You must be signed in to change notification settings - Fork 370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restructure cmake_options.rst
#2230
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that will be much better. I added some suggestion for organizing items a bit differently to provide an even better overview to the user.
@heplesser I think I have addressed all your comments, could you have a second look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stinebuu Thanks a lot! Going through once again I noticed a number of further details, see below. Two general points:
- Should the entries in the left column all be typeset as code, e.g.,
-Dwith-python=[OFF|ON|...]
? - I think it would look tidier to put the bracket with the default into the sentence, i.e., like "Build with MPI parallelization [default: OFF]."
- I also think that the defaults are not all given in the same way, some with :, some with =, some with space.
Co-authored-by: Hans Ekkehard Plesser <[email protected]>
@heplesser Thanks for your comments! I have addressed them now, please have another look. Do you want me to also update the cmake options in Lines 48 to 52 in cb61289
The table widths are quite long now (at least for some of the tables), because I don't know how to cut the links over two lines (the SIONLIB link is the main problem). The width of the External libraries table is now 155 characters. For me this is okay, but I wanted to point it out just in case. |
@stinebuu Yes, having the same information in the CMakeLists.txt would be good. Inconsistency is irritating for users. I think the wide lines should not be a problem, they will be rendered nicely by Sphinx, I believe. @jessica-mitchell, can you confirm this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stinebuu looks good! The large tables render fine as well :)
This fixes #2067.
Convert to tables, and restructure ordering.