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

fix: enumerize with string store accessor keys #441

Merged
merged 1 commit into from
May 22, 2024

Conversation

4ndypanda
Copy link
Contributor

Ensures Enumerize is agnostic to whether the key to a store accessor is a string or symbol.

Re-opening #405 but with fix to not generate string objects with #to_s from original PR. Because ActiveRecord::Store's .store and .store_accessor methods allow string names for attribute names, we should be able to handle them. This has come up for me when dealing with meta-programming where one dynamically defines store attributes.

@4ndypanda 4ndypanda marked this pull request as ready for review January 2, 2024 21:39
@SalvatoreT
Copy link

Hey @nashby, when you get a chance, can you please take a look at this bug fix as well? I think @4ndypanda might need to resolve a conflict, but it looks otherwise good to go!

@4ndypanda 4ndypanda force-pushed the ricksong/fix-store-attributes branch 3 times, most recently from 10d19da to 816767f Compare May 21, 2024 17:35
@4ndypanda
Copy link
Contributor Author

Note to self, rebase after merging #450.

Ensures Enumerize is agnostic to whether the key to a store accessor is a
string or symbol
@4ndypanda 4ndypanda force-pushed the ricksong/fix-store-attributes branch from 816767f to 2957576 Compare May 22, 2024 15:06
@nashby nashby merged commit 95a482f into brainspec:master May 22, 2024
98 checks passed
@nashby
Copy link
Member

nashby commented May 22, 2024

@4ndypanda thanks! I pushed a small change c77d0e3 since Attribute#name is always a symbol.

@4ndypanda 4ndypanda deleted the ricksong/fix-store-attributes branch May 22, 2024 19:55
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.

4 participants