Skip to content

Updates to README.md#37

Open
TumeloN1 wants to merge 1 commit into
gt-tinker:mainfrom
TumeloN1:main
Open

Updates to README.md#37
TumeloN1 wants to merge 1 commit into
gt-tinker:mainfrom
TumeloN1:main

Conversation

@TumeloN1

Copy link
Copy Markdown

Updated the readme.md to include Homebrew installation options for packages.
Added a note to the maturin develop command listing -vvv as being for extended troubleshooting purposes as -vv or -v would likely be sufficient for most errors.

@ausbin ausbin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for making these changes.

Would you making some formatting tweaks?

Also, based on my limited experience with macOS, there are some steps that I don't remember needing to do. Could you confirm? It's worth the effort to keep the dev setup steps as short as we can.

Comment thread README.md
Comment on lines +74 to +76
```
$ 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.
```

Copy link
Copy Markdown
Member

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 export code block)

Comment thread README.md
**macOS (Homebrew):**
```
$ xcode-select --install
$ brew install cmake ninja zlib llvm

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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?

Comment thread README.md
```
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"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't be needed given step 1, right?

Comment thread README.md

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 gq, but I can't speak for other editors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants