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

NOISSUE - Clean up error handling, remove unwrap() calls, logging #188

Merged

Conversation

jmcconnell26
Copy link
Contributor

meaningful messages

@anderejd
Copy link
Contributor

Sorry for the delay, it seems I missed the notification. I would like add you as maintainer if you are interested? @tarcieri would also need to approve.

I'll review this PR now.

Comment on lines -35 to -46
pub trait GetPackageNameFromCargoMetadataPackageId {
fn get_package_name_from_cargo_metadata_package_id(
pub trait GetPackageNameAndVersionFromCargoMetadataPackageId {
fn get_package_name_and_version_from_cargo_metadata_package_id(
&self,
package_id: &cargo_metadata::PackageId,
) -> Option<String>;
}

pub trait GetPackageVersionFromCargoMetadataPackageId {
fn get_package_version_from_cargo_metadata_package_id(
&self,
package_id: &cargo_metadata::PackageId,
) -> Option<cargo_metadata::Version>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice simplification. 👍

@anderejd
Copy link
Contributor

The new error messages look great!

@anderejd anderejd merged commit 22ab44f into geiger-rs:master Feb 12, 2021
@jmcconnell26
Copy link
Contributor Author

Hi @anderejd, I'd definitely be keen to be added as a maintainer!

@anderejd
Copy link
Contributor

@tarcieri Since this project now exists under rust-secure-code it seems reasonable to me to have you on board with adding another maintainer. Your thoughts?

@anderejd
Copy link
Contributor

@tarcieri Ping

@anderejd
Copy link
Contributor

@jmcconnell26 Invite has been sent!

@jmcconnell26
Copy link
Contributor Author

Awesome, accepted it now!

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