-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add build documentation to README.dev.md #113
Conversation
README.dev.md
Outdated
@@ -66,7 +126,7 @@ After a pull request is created, a Codacy test should appear. Follow the link th | |||
|
|||
#### pre-commit hooks |
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.
#### pre-commit hooks | |
#### pre-commit hooks (optional) |
README.dev.md
Outdated
- `gcc` 9 or above | ||
- `g++` 9 or above | ||
- `make` 4 or above | ||
- `cmake` 3.22 or above |
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.
- `cmake` 3.22 or above | |
- `cmake` 3.17 or above |
|
||
For instance, the commands below download install CMake 3.22. | ||
|
||
```shell |
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.
can we just point to official documentation at https://cmake.org/install/ ?
README.dev.md
Outdated
sudo make install | ||
``` | ||
|
||
If you can't run `sudo` to install `cmake` - for instance if you are in a cluster without privileges - you should add the path to the CMake `bin` folder to your path. |
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.
Normal user needs to use CMAKE_INSTALL_PREFIX
flag. Can we point to the official documentation at https://cmake.org/cmake/help/v3.22/variable/CMAKE_INSTALL_PREFIX.html?
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.
Lookg good @abelsiqueira . I made some suggestions.
Hi @fdiblen, thanks for the review. I have updated the pull request with your comments. |
679512a
to
4e7e436
Compare
I have updated again |
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.
Looks good @abelsiqueira Thanks a lot! 🎖️
Thanks for reviewing 🏟️ |
Description
Add developer documentation to be able to build the library.
Related issues:
Instructions to review the pull request
Review online.