docs: Clarify setup for "Rust Components" tutorial#218
docs: Clarify setup for "Rust Components" tutorial#218kate-goldenring merged 17 commits intobytecodealliance:mainfrom
Conversation
|
Hey @mkatychev would you mind taking another look at this? One of the thing we've wanted to do for a bit was refactor to use the same WIT files everywhere, and we finally have that in now for all the guides -- I think the changes here are in conflict with that, but it should simplify your diff as well. |
vados-cosmonic
left a comment
There was a problem hiding this comment.
LGTM 🚀
One nit about an alert but this looks good, let's get this merged :)
Co-authored-by: Victor Adossi <[email protected]>
|
@kate-goldenring this PR should be ready for a final review & merge |
kate-goldenring
left a comment
There was a problem hiding this comment.
Thank you @mkatychev for these updates. My main suggestion is to not require the reader to clone the repository. Rather, have them just fetch the WIT file.
| ```toml | ||
| [package.metadata.component.target.dependencies] | ||
| "docs:adder" = { path = "../adder/wit" } # directory containing the WIT package | ||
| "docs:adder" = { path = "../wit/adder" } # directory containing the WIT package |
There was a problem hiding this comment.
This assumes they cloned the repo rather than just fetching the wit. I'd prefer to stay with the approach of fetching the one wit file rather than cloning the repo
There was a problem hiding this comment.
Following up on #218 (comment), if we don't specify cloning the repo then we'd have to specify fetching the wit/adder, wit/calculator, and examples/adder directories explicitly, I don't mind doing this but it seems like that adds more complexity than a git clone.
git clone is a temporary approach regardless until #241 (comment) is done.
There was a problem hiding this comment.
I think fetching a file is the most analogous to what the wkg experience will be like
There was a problem hiding this comment.
Agreed that that's what we should aim for, however, calling cargo run --release -- 1 2 ../add/target/wasm32-wasip1/release/adder.wasm for this step requires us to have all seven files from the example-host directory present.
Should I instead add an instruction to wget the directory and extract it?
There was a problem hiding this comment.
I agree that the example host is an exception -- for now, they will need to clone the repo to run it. Maybe this line should be cargo run --release -- 1 2 adder.wasm to not imply that they build the adder component from the repo.
The main thing i am thinking about is that the fact that the example host is not fetchable should not define how we instruct users to build components. We can push the host up in a container for example or release it.
There was a problem hiding this comment.
I think it's a great and useful example that demonstrantes using external logic to execute the user's code.
I haven't found a succinct way, other than git, to fetch the example-host logic unless something like wkg can do it.
There was a problem hiding this comment.
Does it make sense to have a wit in ghcr that they can depend on? Then there would be no question about file paths because they wouldn't be needed when specifying the dependency.
There was a problem hiding this comment.
That is definitely the plan: upload the packages to ghcr and update the tutorial and guides to use wkg. We just need the work to be prioritized
There was a problem hiding this comment.
In the meantime for this PR, should I revert the path changes and references to git clone?
EDIT: that's what was done
14098df to
9102bc9
Compare
kate-goldenring
left a comment
There was a problem hiding this comment.
Thank you for making this a smoother experience!
This PR aims to make explicit some assumptions made in the Rust Components tutorial.
Changes made to
component-model/src/language-support/rust.md:example:component/exampleto help disambiguate between similar sounding names such asaddandaddercomponent-docs/component-model/src/language-support/rust.md
Lines 51 to 53 in 2e207e4
$from shell commands to make them easier to copy