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

Remove awrence dependency and test against 3.4 with frozen strings #436

Merged
merged 4 commits into from
Nov 1, 2024

Conversation

npezza93
Copy link
Contributor

Avoids monkey patching Hash and removes a dependency on an external gem.
This is a simpler version of the rails' camelize and deep_transform_keys

@npezza93 npezza93 changed the title Remove awrence dependency Remove awrence dependency and test against 3.4 with frozen strings Oct 23, 2024
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.

Hi @npezza93! Love it! Thanks! ❤️

We were thinking about removing this dependency for a while now, so we really appreciate this! 😄

Left a couple of comments to discuss.

BTW should we add some tests?

lib/webauthn/camelize.rb Outdated Show resolved Hide resolved
lib/webauthn/camelize.rb Outdated Show resolved Hide resolved
@npezza93
Copy link
Contributor Author

BTW should we add some tests?

I can if you want but theres a bunch that already basically check what the key is/what the value of a cameled key is so thought it might be redundant. Let me know!

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.

BTW should we add some tests?

I can if you want but theres a bunch that already basically check what the key is/what the value of a cameled key is so thought it might be redundant. Let me know!

Nevermind! This looks good so I think we should merge it. I can create some specs in a follow up PR 🙂

Then again, thank you so much for taking the time to tackle this!

@santiagorodriguez96 santiagorodriguez96 merged commit b18cccd into cedarcode:master Nov 1, 2024
10 checks passed
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.

2 participants