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

added documentation for public parts of the crate #27

Merged
merged 2 commits into from Nov 3, 2017

Conversation

Projects
None yet
3 participants
@serejkus
Copy link

serejkus commented Oct 31, 2017

Added docs for public parts of the crate, as requested in #17 and #23.

English is not my native language, so feel free to fix grammar issues. The same applies to examples: most of them are just copy-paste and involve parsing of semver structs from strings, maybe there might be a better approach in some cases.

@serejkus

This comment has been minimized.

Copy link
Author

serejkus commented Nov 2, 2017

@KodrAus, any suggestions on this PR?

@KodrAus

This comment has been minimized.

Copy link

KodrAus commented Nov 2, 2017

Sorry I somehow missed this. Thanks for the ping @serejkus and for taking on these docs! I'll dig in to this later today.

@KodrAus
Copy link

KodrAus left a comment

Thanks @serejkus this is an awesome effort! I've just left some comments for a coupe of typos.

src/lib.rs Outdated
//! collection of [`range::Predicate`]
//! representing data for version comparision.
//!
//! # Example

This comment has been minimized.

@KodrAus

KodrAus Nov 3, 2017

Let's change this to Examples to be consistent.

This comment has been minimized.

@steveklabnik

steveklabnik Nov 3, 2017

Owner

In general, all of them should always be Examples :)

src/lib.rs Outdated
@@ -1,3 +1,52 @@
//! Collection of structures and helper functions for parsing semantic version.
//!
//! This crate contains data structures for holding version data and comparision of versions

This comment has been minimized.

@KodrAus

KodrAus Nov 3, 2017

Typo here: comparision to comparison. I always type comparision for some reason...

Looks like there are a few other places we should replace comparision with comparison.

src/lib.rs Outdated
//! use semver_parser::version;
//!
//! # fn try_main() -> Result<(), String> {
//!

This comment has been minimized.

@KodrAus

KodrAus Nov 3, 2017

We should remove these newlines after hidden lines, it makes the code here look a little funny, but when it renders in the docs you only have 1 visible newline.

Does that make sense?

src/lib.rs Outdated
//!
//! # Example
//!
//! ```rust

This comment has been minimized.

@KodrAus

KodrAus Nov 3, 2017

For the docs we don't actually need the rust annotation here. rustdoc assumes the code block contains testable Rust by default.

@@ -1,30 +1,222 @@
//! Version range and requirements data and functions (used for version comparision)

This comment has been minimized.

@KodrAus

KodrAus Nov 3, 2017

Let's throw a . at the end of these introduction lines in all the modules, structures and methods to be consistent.

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct Version {
/// major version as number (`0` in `"0.1.2"`);

This comment has been minimized.

@KodrAus

KodrAus Nov 3, 2017

Let's capitalise the major, minor etc on these docs and use a . instead of a ; at the end.

@serejkus

This comment has been minimized.

Copy link
Author

serejkus commented Nov 3, 2017

Thanks for pointing out! Will try to fix those issues in a day.

@steveklabnik

This comment has been minimized.

Copy link
Owner

steveklabnik commented Nov 3, 2017

Seconded, thanks a ton @serejkus :)

@serejkus

This comment has been minimized.

Copy link
Author

serejkus commented Nov 3, 2017

Fixed issues with style and added links to cargo docs explaining caret and tilde version requirements. Two commits are ok or is it better to rebase it into a single commit?

@steveklabnik

This comment has been minimized.

Copy link
Owner

steveklabnik commented Nov 3, 2017

Two commits is just fine. Thanks again!

@steveklabnik steveklabnik merged commit f3af344 into steveklabnik:master Nov 3, 2017

@serejkus serejkus deleted the serejkus:docs branch Nov 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.