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.

Sign up for GitHub

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

Jump to bottom

Allow any String as Key Use #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow any String as Key Use #9

wants to merge 1 commit into from

Conversation

maxlambrecht
Copy link

@maxlambrecht maxlambrecht commented Mar 10, 2022 •
edited
Loading

Solves #8

@maxlambrecht maxlambrecht mentioned this pull request Mar 10, 2022
Open
@nhynes
Copy link
Owner

nhynes commented Apr 21, 2022

This looks good. Thanks for the PR. I proposed a few modifications around keeping KeyUse as an enum so that the two named in the spec are more convenient and harder to mis-type. Let me know if this interface will work for you.

Either option here is a breaking change, so this'll need to go out with a v0.4.* of this crate. I'd like to batch it up with some changes I'm about to make, if you wouldn't mind waiting. If it's a priority for you, I can make a fresh v0.4.0 with just this change.

nhynes
nhynes reviewed Apr 21, 2022
View reviewed changes
src/lib.rs
Encryption,
Custom(String),
Copy link
Owner

Choose a reason for hiding this comment

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

I'd also like to sprinkle &'a str and whatnot around this code as part of the v0.4 release. After using this crate a bunch, all the allocation is starting to get to me!

@maxlambrecht
Copy link
Author

maxlambrecht commented Apr 21, 2022 •
edited
Loading

This looks good. Thanks for the PR. I proposed a few modifications around keeping KeyUse as an enum so that the two named in the spec are more convenient and harder to mis-type. Let me know if this interface will work for you.

Looks good to me! It makes sense to keep the enum types for those two uses mentioned in the spec.

Either option here is a breaking change, so this'll need to go out with a v0.4.* of this crate. I'd like to batch it up with some changes I'm about to make, if you wouldn't mind waiting. If it's a priority for you, I can make a fresh v0.4.0 with just this change.

No worries, I can wait.

Thanks!

Allow any String as Key Use
3f7d7ce 
Signed-off-by: Max Lambrecht <[email protected]>
@maxlambrecht
Copy link
Author

Hi @nhynes, do you have an ETA for this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhynes nhynes nhynes left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maxlambrecht @nhynes