-
Notifications
You must be signed in to change notification settings - Fork 6
Description
First off, I just followed the README and it worked quite smoothly most of the time. A few minor points here to avoid any confusion. These are all comments on README.md:
- Missing Requirements
I had these on my machine, but you need minimally docker, docker compose, and recent cargo installed. It would be good to cover these as requirements. If you explain how to rustup to stable and add cargo binstall command, then you can just use it below. When doing make wasi-build, I got:
error: rustc 1.81.0 is not supported by the following package:
wavs-wasi-chain@0.3.0-alpha4 requires rustc 1.84.0
I was on rust 1.81 and had to do this to upgrade
# these are only needed for me, with older builds
rustup target remove wasm32-wasi
rustup target remove wasm32-wasip1
# everyone will need this
rustup update stable
rustup target add wasm32-wasip2Also I assume everything is meant for MacOS or Linux, and will not work on Windows (or needs WSL). You should add a comment about that, or test that it does work on Windows. (I assume this is a lot of work and not needed, but direct them to WSL)
- Install step fails if copied verbatum (maybe fixed if you add more to requirements setup)
> Install `cargo binstall cargo-component` if you have not already. -- https://github.com/bytecodealliance/cargo-component#installation
Most people do not have binstall on their system. I would recommend either explaining how to install cargo binstall, or just referencing the standard command: cargo install cargo-component --locked (from their README)
3. The make wasi-exec line fails.
It needs (Fixed in #51 )cp .env.example .env to be run first, but that line is right below it in the README. Maybe you can move that up higher?
Also, I don't have the docker group and need to run (Fixed in #52 )sudo for docker to run properly. I had other scripts that did this check before, could we add it? To prepend sudo if the user is not in group docker? For now, I just added sudo before the various docker calls in the Makefile and that works.
- Minor note after
make start-all
Tell them that this must keep running in that terminal, and they should use another terminal for all the following actions
Other than these relatively minor issues, the whole flow worked well. Very nice achievement to pull together so many tools and so much power into an easy to use walk-through.