Support new ZADD options introduced at Redis 3.0.2 #1083
Conversation
* Please refer http://redis.io/commands/zadd for more details * You can see its functionalities from newly added unit tests
* that it requires Redis 3.0.2 and onwards * also fix formatting
|
Looks great. Thanks for handling all of this, sorry that it had to turn into such a fuss... Looking forward to the *Param args as well. |
|
|
||
| public void zaddBinary(final byte[] key, final Map<byte[], Double> scoreMembers) { |
There was a problem hiding this comment.
I know it has nothing to do with this PR. But I was looking at the zaddBinary method and I was thinking: "why is it called zaddBinary if zadd is already binary?". Then I realized that the difference is that zaddBinary accepts multiple score members.
Maybe we can take the opportunity to rename this method to just zadd?. In Client.java the method that calls zaddBinary is called zadd.
There was a problem hiding this comment.
@marcosnils
Great finding! Renamed zaddBinary to zadd.
Maybe it was just a workaround if Java compiler or JVM could not distinguish methods if only param's Generic is different.
For now, all tests passed.
|
@HeartSaVioR PR is perfect. Let's see what we want to do with the |
|
@HeartSaVioR LAGTM (Looks amazingly good to me??) 👍 |
|
@jamespedwards42 You don't need to be sorry. It was me who can't stand with that decision. :) |
Conflicts: src/main/java/redis/clients/jedis/JedisCluster.java
Support new ZADD options introduced at Redis 3.0.2
|
Merged into master and 2.8 branch respectively. |
Closes #1067
We discussed many things from #1067, so please refer that issue for more details.
I didn't add javadoc for added methods for some reasons.
I'll address what thing prevents me to add javadoc to #978.
Please review and comment. Thanks!