-
Notifications
You must be signed in to change notification settings - Fork 7
Updates to README.md #37
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,12 +68,21 @@ You need to install [Rust][1] _and_ the following: | |
| ``` | ||
| You should set these persistently, e.g., in your `~/.bashrc` or `~/.zshrc`. | ||
|
|
||
| 2. The following Debian/Ubuntu packages are also needed to build: | ||
| ``` | ||
| 2. The following packages are also needed to build: | ||
|
|
||
| **Debian/Ubuntu:** | ||
| ``` | ||
| $ sudo apt install build-essential cmake ninja-build zlib1g-dev libclang-dev | ||
| ``` | ||
| If you are on macOS, installing the XCode developer tools is enough and the | ||
| command above is not needed. | ||
| ``` | ||
| **macOS (Homebrew):** | ||
| ``` | ||
| $ xcode-select --install | ||
| $ brew install cmake ninja zlib llvm | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not rely on the Homebrew LLVM build in my opinion, particularly when step 1 is installing LLVM. I thought XCode shipped with CMake, ninja, and zlib. Does it not? |
||
| ``` | ||
| If you are on macOS, the XCode developer tools (`xcode-select --install`) are required in place of `build-essential`. You may also need to export the Homebrew LLVM path: | ||
| ``` | ||
| $ export PATH="$(brew --prefix llvm)/bin:$PATH" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This shouldn't be needed given step 1, right? |
||
| ``` | ||
|
|
||
| ### Building | ||
|
|
||
|
|
@@ -84,10 +93,10 @@ To generate the Python extension, run the following: | |
| $ . venv/bin/activate | ||
| $ cd qwerty_pyrt | ||
| $ pip install maturin numpy | ||
| $ maturin develop -vvv | ||
| $ maturin develop -vv | ||
|
|
||
| The last command will (re)build _everything_. Passing `-vvv` to | ||
| `maturin develop` helps to give you an idea of what is going on. | ||
| The last command will (re)build _everything_. If you need additional verbosity for troubleshooting, passing `-vvv` to | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for tweaking this. I try to wrap the text in this README at around 80 columns, could you do that too here? Vim can do this with |
||
| `maturin develop` helps to give you a better idea of what is going on. | ||
|
|
||
| ### Testing | ||
|
|
||
|
|
||
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 you keep the indentation of the triple backticks how they were? See the formatting of the previous step in this list for what I mean (the
exportcode block)