Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd "Parse a complex version string" example #308
Conversation
FaultyRAM
changed the title
Parse a complex version string.
Add "Parse a complex version string" example
Oct 1, 2017
budziq
requested changes
Oct 1, 2017
|
@FaultyRAM very nice! I would suggest 2 minor stylistic adjustments and we are good to merge |
| let version_str = "1.0.49-125+g72ee7853"; | ||
| let version = Version::parse(version_str)?; | ||
| assert_eq!(version.major, 1); |
This comment has been minimized.
This comment has been minimized.
budziq
Oct 1, 2017
Collaborator
I would suggest to group these assert_eq's into a single one like in the more basic "Parse and increment a version string" example for the sake of consistency.
| Identifier::AlphaNumeric(String::from("g72ee7853")) | ||
| ); | ||
| let version_string = version.to_string(); |
This comment has been minimized.
This comment has been minimized.
budziq
Oct 1, 2017
Collaborator
i would rename the variable to serialized_version or something along these lines to emphasize what is going on here.
budziq
requested changes
Oct 2, 2017
|
Awesome! Please see to the last nit, squash the commits and we are ready to merge :) |
| ); | ||
| let parsed_version = Version::parse(version_str)?; | ||
| let constructed_version = Version { |
This comment has been minimized.
This comment has been minimized.
budziq
Oct 2, 2017
Collaborator
lets drop the intermediate variable and create it directly in the assert_eq
| build: vec![], | ||
| }; | ||
| assert_eq!(parsed_version, constructed_version); | ||
This comment has been minimized.
This comment has been minimized.
budziq
Oct 2, 2017
Collaborator
lets reintroduce the separate assert_eq with just the build metadata to show that it is in fact parsed.
FaultyRAM
force-pushed the
FaultyRAM:ex-semver-complex
branch
from
3ccef37
to
e913739
Oct 3, 2017
budziq
merged commit b7ab129
into
rust-lang-nursery:master
Oct 4, 2017
This comment has been minimized.
This comment has been minimized.
|
Nicely done @FaultyRAM ! |
FaultyRAM commentedOct 1, 2017
Fixes #284