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

Refactor jws #143 #162

Merged
merged 5 commits into from
May 7, 2024
Merged

Refactor jws #143 #162

merged 5 commits into from
May 7, 2024

Conversation

KendallWeihe
Copy link
Contributor

@KendallWeihe KendallWeihe commented May 6, 2024

Refactor the jws developer surface in accordance with #143

This PR creates the changes in a new v2 module, because there are downstream impacts to the jwt and credentials 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:

@nitro-neal
Copy link
Contributor

working on this to compliment this draft - #149

@KendallWeihe KendallWeihe marked this pull request as ready for review May 6, 2024 21:28
Copy link
Contributor

@kirahsapong kirahsapong left a 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! 🔥

@@ -27,6 +29,10 @@ pub enum JwsError {
AlgorithmNotFound(String),
#[error(transparent)]
CryptoError(#[from] CryptoError),
#[error("serde json error {0}")]
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor

@nitro-neal nitro-neal left a 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

@KendallWeihe
Copy link
Contributor Author

moving fast and (not) breaking things (because we v2'd it)

@KendallWeihe KendallWeihe merged commit 918d0ee into main May 7, 2024
9 checks passed
@KendallWeihe KendallWeihe deleted the kendall/jws-refactor-143 branch May 7, 2024 12:00
This was referenced May 7, 2024
enmand pushed a commit to enmand/web5-rs that referenced this pull request May 20, 2024
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.

3 participants