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

libz blitz: use question mark instead of unwrap in examples #144

Closed
wants to merge 6 commits into from
Closed

libz blitz: use question mark instead of unwrap in examples #144

wants to merge 6 commits into from

Conversation

muzikmoe
Copy link
Contributor

Rebase pull request #141 on master

Copy link

@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.

Thanks for rebasing this @muzikmoe. I've just a comment about the newlines before hidden lines in the doc tests.

src/lib.rs Outdated
//! bugfix_release.increment_patch();
//!
//! assert_eq!(Ok(bugfix_release), Version::parse("1.0.1"));
//!
Copy link

Choose a reason for hiding this comment

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

Let's remove these extra newlines before the hidden lines. Otherwise the docs will have blank lines at the end of the code blocks.

Copy link

@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.

This looks good to me! Thanks again!

@muzikmoe
Copy link
Contributor Author

Thank for your guiding.

src/lib.rs Outdated
@@ -158,7 +186,8 @@
//! ```

#![doc(html_logo_url = "https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png",
html_favicon_url = "https://www.rust-lang.org/favicon.ico")]
html_favicon_url = "https://www.rust-lang.org/favicon.ico")]
#![doc(html_root_url = "https://docs.rs/semver/0.9.0")]
Copy link
Contributor

Choose a reason for hiding this comment

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

this change looks unrelated?

src/lib.rs Outdated
@@ -70,35 +70,56 @@
//!
//! ```{rust}
//! use semver::Version;
//! use std::error::Error;
Copy link

Choose a reason for hiding this comment

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

Ah, sorry I didn't pick this up before! We should probably hide these use std::error::Error lines too since they don't actually relate to the example.

I would probably just remove that import altogether and change the function signatures to:

# fn try_increment_patch() -> Result<(), Box<::std::error::Error>> {

@opilar
Copy link
Contributor

opilar commented Nov 23, 2017

It's in conflict now. @muzikmoe if you're busy, I can resolve it for you.

@muzikmoe
Copy link
Contributor Author

@opilar thank you.

@steveklabnik
Copy link
Contributor

My comment about the unrelated change is still here, and it still needs a rebase. @muzikmoe any interest in keeping up with this PR?

@steveklabnik
Copy link
Contributor

Triage: seems like this PR got lost in the shuffle; I'm going to give it a close. Thanks for the effort.

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.

None yet

4 participants