String identifiers #13

Merged
merged 2 commits into from Jan 1, 2013

2 participants

@chrislloyd

As discussed in jnunemaker/flipper-redis#3 this changes identifiers to strings.

chrislloyd added some commits Nov 21, 2012
@chrislloyd chrislloyd Uses strings as actor identifiers.
Patch related to discussion at  jnunemaker/flipper-redis#3 where it was decided that internally Flipper needs to store id's as strings. This is because some adapters (redis) are typeless blob stores. This change also helps with other datastore compatibility (i.e. Mongo where id's are non-integer).
3a2f57e
@chrislloyd chrislloyd Adapter specs now use string ids instead of integers. 8bb9ae4
@chrislloyd chrislloyd referenced this pull request in jnunemaker/flipper-redis Nov 21, 2012
Merged

Fixes incorrect set coercion. #3

@chrislloyd

BTW, pushing Flipper into Minefold to help roll out some of our new stuff.

@jnunemaker
Owner

That is SO FREAKIN COOL. I'll check these out after the holiday, probably Friday.

@chrislloyd

Might also be worth checking out chrislloyd/flipper@31215b2. Not sure if that's the best place to put the fix though (but it works).

@jnunemaker jnunemaker added a commit that referenced this pull request Jan 1, 2013
@jnunemaker No longer force passing actor instance to enable/disable.
This is related to #13. First, it allows just passing in anything that
responds to id to enable/disable and friends. Second, it wraps whatever
is passed in, if possible, and uses the wrapped value. Third,
identifier is no longer supported. From now on id.to_s will be used
everywhere.
fad3fd0
@jnunemaker jnunemaker merged commit 8bb9ae4 into jnunemaker:master Jan 1, 2013

1 check passed

Details default The Travis build passed
@jnunemaker
Owner

@chrislloyd any thoughts on the commit I just referenced and pushed to master. I'm going to look things over more, but it should fix the remaining issues with string ids. I hope it also makes the API a little easier as you can just pass anything in, instead of an exact actor instance to check if the actor is individually enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment