Skip to content
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

cmake: Recommend native CMake commands in README #1483

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ To maintain a pristine source tree, CMake encourages to perform an out-of-source

$ mkdir build && cd build
$ cmake ..
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Might be explicit:

Suggested change
$ cmake ..
$ cmake -S ..

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yeah, I think shorter is better, and omitting the -S is also more common. (See the official tutorial https://cmake.org/cmake/help/latest/guide/tutorial/A%20Basic%20Starting%20Point.html for example.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, nm.

$ make
$ make check # run the test suite
$ sudo make install # optional
$ cmake --build .
$ ctest # run the test suite
$ sudo cmake --build . --target install # optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth mentioning a modern and more effective alternative, which is available for CMake >= 3.15:

$ sudo cmake --install .  # optional

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. In fact, I had this first, but I changed to the current form because we still support CMake 3.13.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, mention it additionally, not instead of the current line.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, it's OK in its current view.


To compile optional modules (such as Schnorr signatures), you need to run `cmake` with additional flags (such as `-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON`). Run `cmake .. -LH` to see the full list of available flags.

Expand Down
Loading