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 store_boolean_as_native global config option #288

Merged
merged 4 commits into from Aug 15, 2018

Conversation

andrykonchin
Copy link
Member

The option is similar to the store_as_native_boolean field option and means the same.

By default the option is true so any boolean field will be stored as native DynamoDB's boolean value.

@andrykonchin andrykonchin changed the title Add store_boolean_as_native config option Add store_boolean_as_native global config option Aug 14, 2018
@andrykonchin andrykonchin force-pushed the add-store-boolean-as-native-config-option branch from abd5830 to fdff404 Compare August 14, 2018 19:43
Copy link
Member

@pboling pboling left a comment

Choose a reason for hiding this comment

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

Looks great! My comment on constantizing the array ['t', 'f'] is just a nice-to-have.

end
if store_as_boolean
!!value
elsif ['t', 'f'].include?(value)
Copy link
Member

Choose a reason for hiding this comment

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

Would like to make this array a constant within the class. No need to instantiate again on every call.

@andrykonchin andrykonchin merged commit ec9d5f0 into master Aug 15, 2018
@andrykonchin andrykonchin deleted the add-store-boolean-as-native-config-option branch August 15, 2018 18:07
Copy link
Member

@pboling pboling left a comment

Choose a reason for hiding this comment

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

👍

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

2 participants