-
Notifications
You must be signed in to change notification settings - Fork 17
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
Refactor jws #143 #162
Refactor jws #143 #162
Conversation
working on this to compliment this draft - #149 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work you rustacean! 🔥
crates/jws/src/lib.rs
Outdated
@@ -27,6 +29,10 @@ pub enum JwsError { | |||
AlgorithmNotFound(String), | |||
#[error(transparent)] | |||
CryptoError(#[from] CryptoError), | |||
#[error("serde json error {0}")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are taking this whole beautiful lib and throwing it in the trash to make way for v2 after all v2s are ready for the big bang - so these changes dont mean anything eh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's right! the 'ole "v2 this thing temporarily" strategy, which I can understand people not being receptive to, if not we can just hold off merging and string a sequence of PRs together, but I figure since it's a tight nit team working and we plan to do this within the next 2 days might as well keep our process time as low as possible
good catch, I removed these changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merge in and I can add the jws header and more verifications the 'normal' way
moving fast and (not) breaking things (because we v2'd it) |
Refactor the
jws
developer surface in accordance with #143This PR creates the changes in a new
v2
module, because there are downstream impacts to thejwt
andcredentials
crates which will break those. The work for those crates is slated in #144 and #150; we will remove the old (non-v2 code) once that work is completed.This work is ready to be merged. Further work to be done related to this matter:
SerdeJsonError
throughout crates #165