Skip to content
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

Add examples to rustdoc #55

Merged
merged 1 commit into from
Feb 14, 2025
Merged

Conversation

itowlson
Copy link
Contributor

@itowlson itowlson commented Feb 11, 2025

Fixes #34. Further to spinframework/spin#3007.

@kate-goldenring I'm opening this as draft with just a few KV examples done to get feedback on is this the kind of thing you are asking for, or am I missing the mark? If you're happy with the style and approach then I'll press on, otherwise I'd be delighted to iterate with your guidance!

(For folks who want to see how it looks rendered, cargo doc --lib --open.)

As mentioned in spinframework/spin#3007, there may be challenges with adding examples to methods defined in WIT, and it would be useful to understand how folks want to proceed there... Note also that Rust appends the WIT docs at the end of the handwritten docs (after the examples) and I have not yet found an incantation to turn this off!

(ETA: Lann asked for http::send examples - did some of those too.)

(ETA: per Kate's feedback, continuing to build out, a bit ad hoc but eh.)

@itowlson itowlson force-pushed the examples-in-rustdoc branch 3 times, most recently from b6c3d41 to 7cad4c2 Compare February 11, 2025 02:55
@itowlson
Copy link
Contributor Author

itowlson commented Feb 11, 2025

Triggering an error to test CI successfully failed, undid the error

@itowlson itowlson force-pushed the examples-in-rustdoc branch 2 times, most recently from 1aae580 to 7d2ebc0 Compare February 11, 2025 03:07
@kate-goldenring
Copy link
Contributor

This is great @itowlson -- definitely what i was looking for.

There may be challenges with adding examples to methods defined in WIT, and it would be useful to understand how folks want to proceed there

Is the current state of this just that the wit docs are appended? I am seeing docs around spin_sdk::key_value::Store

@itowlson
Copy link
Contributor Author

@kate-goldenring I think there are two things in your message, although they are related.

First off, yes, for re-exports like Store, the WIT docs are appended to the rustdoc. If you look at the bottom of the Store docs you see the original WIT docs sneaking in:

image

It's minor in this case, but for larger WIT docs could be intrusive. And I can't find a way to turn it off for re-exports.

Second, though, and more directly to the bit you quote, we can't add rustdoc to things we don't re-export in the crate (and hence the only doc is the generated WIT doc). In particular, that means we can't override WIT method docs at all. E.g.:

image

In Rust docs this should say Ok(None), and it might be nice to include an example as we do for get_json. But this is a generated method, directly surfaced into the API without explicitly appearing in the SDK code, so there's nowhere to attach rustdoc to it (even if we were okay with having the WIT doc appended).

Does that clarify the difficulty? I feel I might be trying to over-explain and thereby making it even more confusing!

@itowlson itowlson force-pushed the examples-in-rustdoc branch 4 times, most recently from fb4fda6 to 1f31ad0 Compare February 12, 2025 04:57
@kate-goldenring
Copy link
Contributor

@itowlson that does clarify the issue well for me. I think your strategy of highly documenting the struct (Store) and leaving the methods relatively undocumented may be the best path forward

@itowlson itowlson force-pushed the examples-in-rustdoc branch from 1f31ad0 to 00d693f Compare February 13, 2025 01:04
Signed-off-by: itowlson <[email protected]>
@itowlson itowlson force-pushed the examples-in-rustdoc branch from 00d693f to 9229c58 Compare February 13, 2025 03:03
@itowlson itowlson marked this pull request as ready for review February 13, 2025 21:01
@itowlson
Copy link
Contributor Author

Calling this ready for review even though it's a bit patchy - I need to look at some other stuff and I reckon we may as well merge what we have, and bring more of it up to speed over time.

Copy link
Contributor

@kate-goldenring kate-goldenring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @itowlson! Thank you for all of this work. Maybe we can create issues to track adding docs/examples to some of the other parts of the SDK (namely mqtt, pg3, mysql, variables). Those could be great ones for community members to pick up and see this PR as an example

src/key_value.rs Show resolved Hide resolved
@itowlson itowlson merged commit 9e1a51a into spinframework:main Feb 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add examples to doc comments
2 participants