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 crate documentation and update README #38

Merged
merged 5 commits into from
Oct 15, 2018
Merged

Add crate documentation and update README #38

merged 5 commits into from
Oct 15, 2018

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Oct 6, 2018

Closes #16
Closes #7

As discussed, most documentation is in the crate docs and the README is shorter now. The README only shows one simple example and refers to examples/ and the crate docs. All old examples from the old README were added as compile-pass tests.

A note on reviewing: I'm not a native speaker and I'm a perfectionist. So please tell me everything that can be improved. Formulations and stuff that is not common in English and might confuse people should be rewritten. So please don't withhold any comments regarding anything :P

Make sure that we won't delete any useful examples when I will rewrite
the README in the next commit
- Add docs.rs badge
- Remove all old examples and explanation (this is mostly in the crate
  docs now)
- Add a new simple example (the same as in the crate docs)
- Add license section
Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Sorry @LukasKalbertodt I've had a review sitting here for a few days, but must've forgotten to actually submit it!

README.md Outdated
#[auto_impl(Fn)]
trait MyTrait<'a, T> {
fn execute<'b>(&'a self, arg1: &'b T, arg2: &'static str) -> Result<(), String>;
// This will generate two additional impl blocks: one `for &T` and one
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: the ` here is covering the for

src/lib.rs Outdated
//!
//! # Basic syntax and supported types
//!
//! You have to annotate your trait with the `#[auto_impl(...)]` attribute. You
Copy link
Member

Choose a reason for hiding this comment

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

Wording wise it might be better to use can rather than have here, so it reads a bit easier.

src/lib.rs Outdated
//!
//! // All these calls work, as the `#[auto_impl]` attribute generated four
//! // impls for all those proxy types
//! require_animal(Dog("Niko".into()));
Copy link
Member

Choose a reason for hiding this comment

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

:) Not sure about the insinuation here...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I worried about that, too :/
Like, it's obviously meant in a funny/nice way (everyone likes dogs?), but gotta take care not to make others feel strange/bad. Will change all names to "Doggo" ^_^

@LukasKalbertodt LukasKalbertodt merged commit b15c579 into auto-impl-rs:master Oct 15, 2018
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