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 kredis_boolean attribute with default argument #20

Closed
wants to merge 1 commit into from
Closed

Add kredis_boolean attribute with default argument #20

wants to merge 1 commit into from

Conversation

MrHubble
Copy link
Contributor

Add kredis_boolean. Follows in the same vein of existing attributes (e.g. kredis_integer). The only difference being I added default as an argument:

kredis_boolean :noodling, default: false

@kaspth
Copy link
Contributor

kaspth commented Feb 26, 2021

What's the use case? This is generally what the flag type is meant to support, so the boolean aspect is whether or not the key has a value in Redis.

@MrHubble
Copy link
Contributor Author

I didn't realise that flag was meant for this use case.

I'm happy to close this PR if it adds no value.

Thanks for the feedback. I'm a big fan of your work.

@azyzio
Copy link

azyzio commented Mar 3, 2021

Hi,

Thanks for this great encapsulation :) Had to build a custom framework for this kind of things before.

I can actually see a use case for booleans with defaults:

  • we can have several user settings to enable / disable certain features, e.g. reports_enabled, api_enabled, notifications_enabled
  • For consistency, we want to keep all naming to have _enabled suffix
  • By default, we want to keep certain features enabled and others disabled

Right now, we could do it by raising flags in an after_craete callback, but a default option would be nicer.

Also, flags seem to be great for 'flagging' things. E.g. crossed_the_finish_line, invitation_sent. It feels that settings like above are a kind of different category. But this is a minot thing :)

Maybe you can see a better place for settings like those?

Best,
Adam

@MrHubble MrHubble closed this Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants