Kitura / Kitura-redis

Swift Redis library
Apache License 2.0
94 stars 25 forks source link

Added the publish command #44

Closed Herz3h closed 7 years ago

Herz3h commented 7 years ago

Description

Added a pub command to publish on a channel

Motivation and Context

The publish/subscribe system does not exist yet, this is a step into adding part of it

How Has This Been Tested?

I've sent a message on a channel and the python clients who were subscribed to that channel received the message in the expected format.

Checklist:

shmuelk commented 7 years ago

@Herz3h Thank you very much for the PR for the pub command, especially with API documentation. Please follow the instructions here for signing the CLA, we need a signed CLA from you before we can merge your PR.

A couple of technical things:

  1. Please rename the function pub to publish. We have tried to date to name the functions using the "Redis command name".
  2. Please move this new command to a new file "Redis+pubsub.swift". See the other extension files for the style of the file. We are trying to break up the various commands into groups. There will hopefully be eventually more of the pubsub commands.
  3. You may have noticed that for many of the commands we have two flavors, one that takes a value as an ordinary String, as you did, and a second one that takes a RedisString as the value argument. Please add such a function.
  4. Please add tests. Without tests we can not merge this. Add a new file for the tests, don't forget to update the Tests/LinuxMain.swift file.
  5. I would have expected that you would have added at least the subscribe command as well, you probably need this to write tests.