Fixes incorrect set coercion. #3

Merged
merged 2 commits into from Jan 1, 2013

3 participants

@chrislloyd

The to_i was causing group flippers to fail as their name (a string) would incorrectly get coerced into a integer (usually 0).

@chrislloyd chrislloyd Fixes incorrect set coercion.
The "to_i" was causing group flippers to fail as their name (a string) would incorrectly get coerced into a integer (usually 0).
8dbc089
@chrislloyd

Huh seeing the CI failure makes this a bit more of an interesting issue. I'm guessing flipper expects the sets to be integers because of toggling individual users IDs?

@jnunemaker
Owner

Yeah, so I kind of think we need to change flipper itself to take strings instead of integers. Originally I chose integer because I could then use modulo for percentage of actors. Since then, I merged a pull request in flipper to use hashing of the Actor#id.to_s instead of Actor#id. This means we can now force strings for actor id and simplify flipper itself, but I haven't had time to do it.

@chrislloyd chrislloyd referenced this pull request in jnunemaker/flipper Nov 21, 2012
Merged

String identifiers #13

@chrislloyd

Build fails pending acceptance of jnunemaker/flipper#13.

@JackCA

Ran into this same exact issue today and was about to write a fix. The tests in jnunemaker/flipper#13 will indeed cause the current state of this gem to fail. Implementation looks right. Nice work @chrislloyd

@jnunemaker jnunemaker merged commit 4acbe04 into jnunemaker:master Jan 1, 2013

1 check failed

Details default The Travis build failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment