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

Update base64 crate and use it in place of base64-url #871

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

paolobarbolini
Copy link
Contributor

Updates the base64 crate to 0.21 and uses it everywhere for base64 url-safe encoding and decoding.

Let me know if you like the small module I created to wrap the base64 API with something easier to use or if you'd prefer calling base64 directly

@paolobarbolini
Copy link
Contributor Author

Sorry I pushed again as i forgot to run clippy

Copy link
Collaborator

@caspervonb caspervonb left a comment

Choose a reason for hiding this comment

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

Thank you for the effort.

This is more code rather than less, so am not inclined to merge as-is.

Might merge if you remove the base64 helper module, as most of those functions have helpers elsewhere (e.g encode_object_name is just calling encode with URL_SAFE config already.

@paolobarbolini paolobarbolini force-pushed the base64-crate-only branch 2 times, most recently from 0f5371b to b0e9ee2 Compare March 17, 2023 06:38
@paolobarbolini
Copy link
Contributor Author

paolobarbolini commented Mar 17, 2023

Finally got the various different encodings right. I'll remove the base64.rs helper module next and squash everything

@paolobarbolini
Copy link
Contributor Author

This should now be ready to be merged

@caspervonb caspervonb self-requested a review April 3, 2023 13:00
Copy link
Collaborator

@caspervonb caspervonb left a comment

Choose a reason for hiding this comment

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

lgtm, thank you @paolobarbolini

Copy link
Member

@Jarema Jarema left a comment

Choose a reason for hiding this comment

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

LGTM

@Jarema Jarema merged commit 87d7f04 into nats-io:main Apr 5, 2023
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.

None yet

3 participants