chrisdinn / brando

A Redis client written with Akka's IO package
Other
107 stars 24 forks source link

Add pubsub functionality (less change to Request portion of code - fits better with the style) #16

Closed jasongoodwin closed 11 years ago

jasongoodwin commented 11 years ago

After thinking for a bit I'm resubmitting this pull request with some amendments. The Request.scala file has been reverted and the code has been modified to be a bit more in line with the API as it currently sits - that is, you make a Sub request with the 'Request' object as you would any other command. The sender is inferred from context and so sending a message subscribes the sender as one would expect.

I think this is the best approach and keeps the code base a little simpler and is in line with the current design choices.

chrisdinn commented 11 years ago

I like this implemendation a lot. Thanks! However, I can't automatically merge it, there seem to be conflicts. I'll merge once I get a chance to sort them out, unless you'd like to give it a shot.

jasongoodwin commented 11 years ago

Ya I saw that - I think you made some changes after I forked it - I can try to take care of this over the weekend.

Cheers,

Jason Goodwin Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Saturday, 14 September, 2013 at 9:45 AM, Chris Dinn wrote:

I like this implemendation a lot. Thanks! However, I can't automatically merge it, there seem to be conflicts. I'll merge once I get a chance to sort them out, unless you'd like to give it a shot.

— Reply to this email directly or view it on GitHub (https://github.com/chrisdinn/brando/pull/16#issuecomment-24446736).

jasongoodwin commented 11 years ago

Hey Chris, there is a commit that covers the merge there now - let me know if you have any problems. The conflict was on the version in the build.sbt.

The docs may need to be updated once brando is published as well for the version change.

chrisdinn commented 11 years ago

Great, thanks. Yeah, we'll need to get some documentation in there soon.