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

Feat: improved cargo shuttle version error #219

Merged

Conversation

jmwill86
Copy link
Contributor

@jmwill86 jmwill86 commented Jun 20, 2022

Small fix for helping to reduce any initial friction. On firing up a previous project I got the "unexpected end of input while parsing minor version number" due to an old "0.2" version on the shuttle-service. This merge request makes it clearer that the version error is related to the shuttle-service value in Cargo.toml.

Related to #213

Comment on lines +284 to +287
let service_semver = match Version::parse(current_shuttle_version.as_str().unwrap()) {
Ok(version) => version,
Err(error) => return Err(anyhow!("Your shuttle-service version ({}) is invalid and should follow the MAJOR.MINOR.PATCH semantic versioning format. Error given: {:?}", current_shuttle_version.as_str().unwrap(), error.to_string())),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let service_semver = match Version::parse(current_shuttle_version.as_str().unwrap()) {
Ok(version) => version,
Err(error) => return Err(anyhow!("Your shuttle-service version ({}) is invalid and should follow the MAJOR.MINOR.PATCH semantic versioning format. Error given: {:?}", current_shuttle_version.as_str().unwrap(), error.to_string())),
};
let service_semver = Version::parse(current_shuttle_version.as_str().unwrap())
.map_err(|error| anyhow!(
"Your shuttle-service version ({}) is invalid and should follow the MAJOR.MINOR.PATCH semantic versioning format. Error given: {:?}",
current_shuttle_version.as_str().unwrap(),
error.to_string()
))?;

Another common why of structuring this. But a cargo fmt might be needed if you want to accept this suggestion.

@chesedo
Copy link
Contributor

chesedo commented Jun 21, 2022

Btw, thanks for this @jmwill86!

@chesedo chesedo merged commit ed8f259 into shuttle-hq:main Jun 30, 2022
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