-
Notifications
You must be signed in to change notification settings - Fork 33
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 hello world example to use repo on crates.io #51
Conversation
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.
Thank you! It's a good change, we should probably use the opportunity to clean this up a bit, see comments.
src/intro/hello-world.md
Outdated
```log | ||
$ cargo build | ||
Compiling godot4-prebuilt v0.0.0 | ||
(https://github.com/godot-rust/godot4-prebuilt?branch=4.1.1#fca6897d) | ||
Compiling proc-macro2 v1.0.69 | ||
[...] | ||
Compiling godot v0.1.0 (https://github.com/godot-rust/gdext?branch=master#66df8f47) | ||
Compiling godot-cell v0.1.1 | ||
Compiling glam v0.27.0 | ||
Compiling godot-core v0.1.1 | ||
Compiling godot-ffi v0.1.1 | ||
Compiling godot v0.1.1 |
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.
I think this is a good example why we shouldn't include the log. It will already be outdated on next crates.io release again, and anytime we change dependencies, it becomes inaccurate. Furthermore, the output differs depending on the number of crates that have changed (it's not a full rebuild).
Could you remove the whole cargo build
log?
The part about ls -l
is also a bit strange, because ls
doesn't typically show nested directories, nor does it put them in a frame. Maybe we should use a more conventional output here, following a ls -l target/debug
command. But we can also just keep it OS-independent with a paragraph like
After building, you should have the following files in the
target/debug
directory. Note that this applies to Linux, on Windows and macOS you will have different dynamic library names and file extensions.
src/intro/hello-world.md
Outdated
[dependencies] | ||
godot = { git = "https://github.com/godot-rust/gdext", branch = "master" } | ||
godot = "0.1" | ||
``` |
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.
I think we should keep both versions, for users who want a crates release and users who want to follow bleeding-edge development. Maybe mention the crates.io one first, and then introduce the GitHub one.
@kyle-watson Any update? 🙂 |
Can we maybe recommend using |
@kyle-watson Do you have an update on this? |
This is my first ever pull request, so please let me know if I messed anything up or if this isn't even a desired change. Also I am not positive if the Linux log is accurate as I compiled on windows and I replaced the parts of the log that looked similar.