-
Notifications
You must be signed in to change notification settings - Fork 60
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
Update wording on LLVM versions #139
Conversation
✅ Deploy Preview for aya-rs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
docs/book/start/development.md
Outdated
@@ -20,7 +20,7 @@ cargo install bpf-linker | |||
``` | |||
|
|||
If you are running **macos, or linux on any other architecture**, you need to | |||
install LLVM 16 first (for example, with `brew install llvm`), | |||
install the current LLVM first (for example, with `brew install llvm`), |
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.
How about just "LLVM"?
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.
How about just "LLVM"?
I think then it wouldn't be clear that not any version of LLVM will work but one needs the specific / current one.
Maybe we should say install or update LLVM
?
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.
In that case we should say 17.
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.
Or just "the newest stable version". Like, literally, whatever is the latest stable on apt.llvm.org at the moment. And users of non-Debian/Ubuntu distros will understand and figure out what to do. :)
Rust nightly is always switching to new LLVM already when the first rc-s are available. And we are planning to do binstalls anyway. So I don't think that manually editing the doc just to bump the version number is worth an effort.
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.
changed to the newest stable version
and in crosscompile.md
added a mention of and link to the llvm-sys crate and its major version
Signed-off-by: Dmitry S <[email protected]>
No description provided.