Skip to content

Documentation pre-review #219

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

Closed
itowlson opened this issue Mar 23, 2022 · 61 comments
Closed

Documentation pre-review #219

itowlson opened this issue Mar 23, 2022 · 61 comments
Labels
documentation Improvements or additions to documentation

Comments

@itowlson
Copy link
Collaborator

Creating this thread to jot down things I notice as I read the Spin docs.

@itowlson
Copy link
Collaborator Author

Consider a non-animated logo. It's mildly distracting for me, and may be more so for others.

@radu-matei
Copy link
Member

Introduction page on the sidebar should not redirect to /index.

@radu-matei
Copy link
Member

Sidebar should scroll independently of the main content.

@itowlson
Copy link
Collaborator Author

In prose, entry point should be two words.

@radu-matei
Copy link
Member

Current page should be highlighted on the sidebar.

@itowlson
Copy link
Collaborator Author

How early in the docs should we mention "what it works with today"?

@itowlson
Copy link
Collaborator Author

For the quick start, there should be some information about prerequisites. E.g. "If you want to build the example applications yourself, see the Rust language guide for what you'll need."

@itowlson
Copy link
Collaborator Author

Quickstart: "focusing first on spin.toml". Consider a brief explanation of what spin.toml is because the plain filename is a bit out of the blue. E.g. "We'll start by looking at the application definition, which is in the spin.toml file."

@itowlson
Copy link
Collaborator Author

(Longer term - probably too late for 0.1) We might want to consider spin instead of spin_sdk as the crate name for the Rust Spin SDK.

@radu-matei
Copy link
Member

(Longer term - probably too late for 0.1) We might want to consider spin instead of spin_sdk as the crate name for the Rust Spin SDK.

"spin" as a Rust crate is already taken — https://crates.io/crates/spin
Which is why I considered spin_sdk as the public crate.

@radu-matei
Copy link
Member

The Rust language guide should mention the need to add the wasm32-wasi target.

@itowlson
Copy link
Collaborator Author

The quickstart also mentions building the sample component, so would be good to link to that from there.

@itowlson
Copy link
Collaborator Author

I would really like to see some "if something went wrong..." pointers - not necessarily breaking the flow, but just linking off in case people get lost.

@itowlson
Copy link
Collaborator Author

itowlson commented Mar 23, 2022

Configuration: "Spin applications are comprised of general information" - didn't quite know what to make of this.

@itowlson
Copy link
Collaborator Author

The app level fields in the config reference are in a different order to in the sample, which is not a problem but requires more attention to follow along.

@itowlson
Copy link
Collaborator Author

itowlson commented Mar 23, 2022

For the future, maybe, but I would break out the different trigger types and their fields, rather than putting them three levels down in the bullet list.

ETA: We could have the trigger-specific component settings in the same sections. I.e. a section on the HTTP trigger which includes both app and component level settings.

@itowlson
Copy link
Collaborator Author

In config > source, I would make the "not implemented" part of the bindle bit more prominent and not specify the detail e.g.

"[Planned} In future, you'll be able to reference a Wasm module parcel in a remote bindle"

@itowlson
Copy link
Collaborator Author

In config > files, the placement notation example shows an absolute source. The source should be relative.

@itowlson
Copy link
Collaborator Author

In config > WAGI: "${SCRIPT_NAME} will be replaced with the script name". What is 'the script name'?

@itowlson
Copy link
Collaborator Author

The HTTP and Redis triggers section need to come out of the Advanced section and be linked from the config section. The stuff about SCRIPT_NAME and all that being in config risks spreading information across multiple places. Users are going to need a clear, complete reference location.

@itowlson
Copy link
Collaborator Author

There's nothing about creating new applications from templates?

@itowlson
Copy link
Collaborator Author

Is it the case that only one function in a Rust module can have the http_component attribute?

@itowlson
Copy link
Collaborator Author

On the "likely mistakes" front mention that the default spin.toml only picks up release builds.

@itowlson
Copy link
Collaborator Author

Http trigger page: executor = "spin" - is this valid?

@itowlson
Copy link
Collaborator Author

itowlson commented Mar 23, 2022

Http trigger page:

For example, if the application base path is base = /base, and a component has defined route = /foo, that component will be executed for requests on http/s::<spin-up-defined-address-and-port>/base/bar.

  • Should be /base/foo?
  • What is the double colon - is that meant to be :// or is this some IPv6 thing?

@itowlson
Copy link
Collaborator Author

Really make the docs for the various bits of the URL more prominent. There are so many and their names are not intuitive. Brian and I were struggling to figure out the right one for something this morning. Consider a table for the examples.

@itowlson
Copy link
Collaborator Author

Related: we must document the dreaded uri member of the HTTP Request type, with examples. It is NOT what many people understand as a URI (yes, I know the discussion and that other platforms adopt the same convention, but still: if we don't describe it clearly, we will surprise people).

@itowlson
Copy link
Collaborator Author

Tutorial needs a prerequisites link too

@itowlson
Copy link
Collaborator Author

We might need a bit of discussion somewhere about what the WIT files mean, and how they are projected into languages. But that might indeed be an advanced topic.

@radu-matei
Copy link
Member

Here is a first pass at addressing some of the comments — #220

@radu-matei
Copy link
Member

(we will want to revisit a lot of the topics discussed here, but the PR linked here tries to address the most critical things that we want in the short term in the docs)

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

Do we need the layout to be fullwidth for a particular reason? Its very wiiiiiiiiiiiiiiide on external monitor.

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

The prominence and position of the "Edit on GitHub" link is a little intense. Maybe it could live just below the content?

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

"Configuration" page should explicitly mention that the config is TOML (and maybe link to https://toml.io?).

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

wasm32-wasi instructions are duplicated in "Rust" guide:

Besides cargo and the Rust compiler, you need to add the wasm32-wasi target before compiling Rust components for Spin:
In order to compile Rust programs to Spin components, you also need the wasm32-wasi target. You can add it using rustup:

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

Rust guide under "HTTP components" mentions "Rust SDK" but not how to get it as a dependency. Maybe just needs a snippet like:

# Cargo.toml
[dependencies]
spin-sdk = { git = "https://github.com/fermyon/spin/sdk/rust", version = "0.1.0" }

@lann

This comment was marked as resolved.

@lann

This comment was marked as resolved.

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

The Spin SDK for Go uses TinyGo’s WASI support

Slightly confusing: that link doesn't mention WASI

lann added a commit that referenced this issue Mar 24, 2022
@lann
Copy link
Collaborator

lann commented Mar 24, 2022

Given #209 should we spin up -f spin.toml -> spin up everywhere?

@lann
Copy link
Collaborator

lann commented Mar 24, 2022

In Go / Sending outbound HTTP requests, it might be worth calling out explicitly that the stdlib http client doesn't work (and why: #206).

lann added a commit that referenced this issue Mar 24, 2022
lann added a commit that referenced this issue Mar 24, 2022
@lann
Copy link
Collaborator

lann commented Mar 24, 2022

wasm32-unknown-unknown doesn't appear to actually be required any more.

@flynnduism
Copy link
Contributor

@flynnduism
Copy link
Contributor

flynnduism commented Mar 24, 2022

Running into errors on M1 Macbook, noticed the note about openssl in Quickstart wasn't working for me.

I think the note here may be missing something. When I looked in Cellar I noticed my directory structure was 1.1.1n rather than the 1.1.1m mentioned in our docs. I manually copied to /usr/local/openssl-aarch64 and then it worked.

@itowlson
Copy link
Collaborator Author

On the quickstart page, at the end of "Building the example applications", there's a command line to build the sample app. This can go wrong, spectacularly and undiagnosably so if you are a bearded idiot who forgot to install the WASI target.

Could we prominently link to the "common build problems" section from here please?

@itowlson
Copy link
Collaborator Author

In the Rust troubleshooting section, it says

ensure wasm32-wasi target is configured for your Rust installation — you can add it by running rustup target add wasm32-wasi

Should we say how to ensure it's installed (rustup target list --installed)? Or are we just just YOLO ADD IT ANYWAY

@radu-matei
Copy link
Member

YOLO ADD IT ANYWAY

@itowlson
Copy link
Collaborator Author

Rust lang guide - what is my cargo new command line for a Rust component - plain cargo new or cargo new --lib or...?

@itowlson
Copy link
Collaborator Author

Rust lang guide - think these paras were meant to be one, possibly a merge glitch

image

@itowlson
Copy link
Collaborator Author

Rust lang guide - need to know what crate to cargo add (or edit into Cargo.toml)

@itowlson itowlson added documentation Improvements or additions to documentation and removed docs labels May 6, 2022
@radu-matei
Copy link
Member

This was extremely helpful, everyone, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants