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

Add base64 as an inlined implementation #402

Merged

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Sep 22, 2023

This avoids a warning emitted by Ruby 3.3.0-preview2.

Fixes #401.

EDIT: inline the base64 implementation, and leave code comments that remind the reader about what they are seeing there.

Turn the base64 gem into a spec-supporting development dependency.

@santiagorodriguez96
Copy link
Contributor

Hey @olleolleolle! Thank you so much for your contribution!

However, we were thinking about just inlining the implementation of base64 since it's just a thin wrapper and we don't even need all its methods.

Feel free making this adjustment in this PR (or create a new one). Otherwise I will try creating a new PR for the inlining change in the coming week 🙂

This avoids a warning emitted by Ruby 3.3.0-preview2.

Fixes cedarcode#401.
@olleolleolle olleolleolle changed the title Add base64 as a runtime dependency Add base64 as an inlined implementation Oct 13, 2023
@olleolleolle
Copy link
Contributor Author

@santiagorodriguez96 Hope your weekend's good!

Copy link
Contributor

@santiagorodriguez96 santiagorodriguez96 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 so much! ❤️

Have a nice weekend you too!

@santiagorodriguez96 santiagorodriguez96 merged commit d897c1a into cedarcode:master Oct 13, 2023
8 checks passed
@olleolleolle olleolleolle deleted the add-base64-dependency branch October 14, 2023 06:59
@Earlopain
Copy link

I think there's something missing here. The require in the file is still present, and U2fMigrator still calls base64 methods.

@olleolleolle
Copy link
Contributor Author

olleolleolle commented Feb 8, 2024

@Earlopain

@attestation_trust_path ||= [OpenSSL::X509::Certificate.new(Base64.strict_decode64(@certificate))]
Indeed! In order to capture that where it can be seen, Issue #422.

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.

Add base64 dependency to gemspec or inline implementation
3 participants