Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
docs: Clarify setup for "Rust Components" tutorial #218
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?
docs: Clarify setup for "Rust Components" tutorial #218
Changes from 9 commits
2e207e4
d22b430
97d6d55
262d47f
661ac99
d23903d
a2228c0
02e4760
4c1c58c
b8c77c6
1db8a29
5bbe8e4
688bbbd
e63b3d1
917d2b0
ec05fac
9102bc9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
, andexamples/adder
directories explicitly, I don't mind doing this but it seems like that adds more complexity than agit clone
.git clone
is a temporary approach regardless until #241 (comment) is done.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 fetching a file is the most analogous to what the wkg experience will be like
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 likewkg
can do it.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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue to get them in GHCR: #243
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 the meantime for this PR, should I revert the path changes and references to
git clone
?EDIT: that's what was done