Skip to content

Add crate docs / README#11

Merged
alice-i-cecile merged 8 commits intomainfrom
crate-docs
Aug 29, 2024
Merged

Add crate docs / README#11
alice-i-cecile merged 8 commits intomainfrom
crate-docs

Conversation

@alice-i-cecile
Copy link
Copy Markdown
Member

Fixes #7.

@alice-i-cecile alice-i-cecile requested review from cart and mnmaita August 29, 2024 16:22
@alice-i-cecile alice-i-cecile added the documentation Improvements or additions to documentation label Aug 29, 2024
@alice-i-cecile
Copy link
Copy Markdown
Member Author

Note to reviewers: this isn't originally my code, and I haven't worked with it extensively. If something seems wrong to you, please speak up!

Copy link
Copy Markdown

@soqb soqb left a comment

Choose a reason for hiding this comment

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

i think the docs are sound. i just have a few comments/questions !

Copy link
Copy Markdown
Member

@mnmaita mnmaita left a comment

Choose a reason for hiding this comment

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

Leaving a few suggestions!

Copy link
Copy Markdown
Member

@BD103 BD103 left a comment

Choose a reason for hiding this comment

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

Fun trick that I learned from this Linebender blog post:

You can make the header hidden in the generated Rustdoc with a bit of HTML and CSS.

alice-i-cecile and others added 3 commits August 29, 2024 15:00
Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
@alice-i-cecile
Copy link
Copy Markdown
Member Author

Thanks for all the feedback <3

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 29, 2024
Merged via the queue into main with commit cf7ce69 Aug 29, 2024
Copy link
Copy Markdown
Member

@MrGVSV MrGVSV left a comment

Choose a reason for hiding this comment

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

Looks good! I find myself wanting something like this a lot.

My one critique of this crate is that it doesn't seem to support From<T>/Into<T>. I get this is a limitation due to the impl for &'static T, but it does seem to make this a lot harder to use with third-party types.

For example, if Bevy chooses not to implement From<TypeInfo> for CowArc<'static, TypeInfo>, then a given a fn(impl Into<CowArc<'static, TypeInfo>), users would have to have to either do: my_func(&info) or my_func(CowArc::Owned(Arc::new(info))).

At the very least I think we should either support From<Arc<T>> or add a dedicated new_owned constructor.

A [`Cow`](https://doc.rust-lang.org/std/borrow/enum.Cow.html)-like data structure where owned data is stored inside an [`Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html).
Here's what it looks like:

```rust, ignore
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should include an example of usage somewhere here. I find it's often very helpful to see the intended usage of a crate before I start digging through the docs manually.

I think a big selling point of this crate is the automatic 'static preservation when using From/Into. We can probably highlight this in an example.

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

Successfully merging this pull request may close these issues.

Add crate-level docs / README

5 participants