[GitHub] [geode] ringles commented on a change in pull request #6738: GEODE-9186: Add Radish ZPOPMIN command

[GitBox <gi...@apache.org>]

ringles commented on a change in pull request #6738:
URL: https://github.com/apache/geode/pull/6738#discussion_r687068620



##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSetCommandsFunctionExecutor.java
##########
@@ -103,6 +103,12 @@ public long zrevrank(RedisKey key, byte[] member) {
     return stripedExecute(key, () -> getRedisSortedSet(key, true).zscore(member));
   }
 
+  @Override
+  public List<byte[]> zpopmin(RedisKey key, int count) {

Review comment:
       Another call for alphabetization. :)

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/sortedset/RedisSortedSetCommands.java
##########
@@ -46,5 +46,7 @@
 
   byte[] zscore(RedisKey key, byte[] member);
 
+  List<byte[]> zpopmin(RedisKey key, int count);

Review comment:
       Gimme an alpha! Gimme a beta! What does that spell? Alphabeta, which is almost 'alpahbet'...

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/RedisSortedSet.java
##########
@@ -401,10 +401,22 @@ long zrevrank(byte[] member) {
     return null;
   }
 
+  List<byte[]> zpopmin(Region<RedisKey, RedisData> region, RedisKey key, int count) {

Review comment:
       Again, we should probably stick to alphabetical order here. Though that means both zpop*'s should go above zscore...

##########
File path: geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/data/NullRedisSortedSet.java
##########
@@ -86,6 +86,11 @@ long zcount(SortedSetScoreRangeOptions rangeOptions) {
     return increment;
   }
 
+  @Override
+  List<byte[]> zpopmin(Region<RedisKey, RedisData> region, RedisKey key, int count) {

Review comment:
       I think this should go below "zpopmax" alphabetically.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org