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 code examples to lib.rs #57

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

austin-denoble
Copy link
Contributor

@austin-denoble austin-denoble commented Sep 3, 2024

Problem

We got feedback from @spacejam that it would be nice to add more detailed documentation and code examples to the top level lib.rs file to improve what people are shown when they hit the create on docs.rs: https://docs.rs/pinecone-sdk/latest/pinecone_sdk/index.html

Solution

I'm still pretty new to Rust and getting my bearings with a lot of things. Any feedback around best practices for presenting code or information through the crate and rustdoc, please let me know.

  • Added some minimal code examples to the top level lib.rs file focused on some of the basic operations that can be done with the client.
  • The README.md code examples were a bit out of date with some changes we've made - also did some clean up in there.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

Test Plan

I tested all of the code added to lib.rs in a local project, so I at least know it runs and compiles.


@@ -19,12 +21,16 @@ Run `cargo add pinecone-sdk` to add the crate as a dependency, or add the line `
## Usage

The `PineconeClient` class is the main point of entry into the Rust SDK. Parameters may either be directly passed in as `Options`, or read through environment variables as follows. More detail:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The spam of line breaks in here seems like it's from my vscode format-on-save settings. I think adding the spaces in is helpful visually.

```rust
use pinecone_sdk::pinecone::PineconeClientConfig;
use pinecone_sdk::pinecone::control::{Metric, Cloud, WaitPolicy, IndexModel};
use pinecone_sdk::models::{Metric, Cloud, WaitPolicy, IndexModel};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think most of the code examples in here need this update. I can't remember if the types were originally exported from the modules and that was changed, but I needed to write things differently when I was testing locally.

@@ -12,6 +152,12 @@ pub mod pinecone;
/// Utility modules.
pub mod utils;

/// Models for the Pinecone SDK.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just moved these up to group the public stuff together.

@austin-denoble austin-denoble marked this pull request as ready for review September 4, 2024 00:25
@haruska
Copy link

haruska commented Sep 5, 2024

This is good cleanup. A benefit to having the examples in lib.rs is you can actually run the examples and add assert! statements to them as a part of cargo tests. That way they never get out of date.

The README.md shows up on crates.io: https://crates.io/crates/pinecone-sdk

The lib.rs docs show up on docs.rs: https://docs.rs/pinecone-sdk/latest/pinecone_sdk/

We might want to consider just linking to docs.rs in the README for most of the code examples and keep the example code docs in lib.rs as a part of the CI cargo tests to ensure the examples continue to work.

A good example might be axum:

Copy link

@haruska haruska left a comment

Choose a reason for hiding this comment

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

Good cleanup. See previous comment for consideration.

@austin-denoble
Copy link
Contributor Author

Good cleanup. See previous comment for consideration.

Appreciate the callouts, I agree regarding following up and moving our code examples into the lib.rs explicitly. Created a ticket for tracking.

@austin-denoble austin-denoble merged commit e8eea4a into main Sep 6, 2024
2 checks passed
@austin-denoble austin-denoble deleted the adenoble/add-lib-minimal-examples branch September 6, 2024 17:53
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.

2 participants