DonalEvans commented on a change in pull request #6716:
URL: https://github.com/apache/geode/pull/6716#discussion_r675156262
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/netty/StringBytesGlossary.java
##########
@@ -175,12 +175,13 @@
@MakeImmutable
public static final byte[] bABSTTL = stringToBytes("ABSTTL");
- // AbstractZRangeExecutor
+ // Various ZRangeExecutors
@MakeImmutable
public static final byte[] bWITHSCORES = stringToBytes("WITHSCORES");
-
@MakeImmutable
public static final byte[] bLIMIT = stringToBytes("LIMIT");
+ public static final byte bPLUS = SIMPLE_STRING_ID; // +
Review comment:
Prior to the refactor that moved constants out of `Coder`, the approach
to naming byte/byte array constants was somewhat inconsistent, with some using
the "b" prefix and some not. I can't say why this naming convention was
originally chosen (commit history shows that it seems to have been present
since Geode was first imported to git back in 2015), but when working on
https://issues.apache.org/jira/browse/GEODE-9287, I tried to enforce
consistency in all the constant names. I think I was overzealous with the
single-byte values though, as you're right, many of them don't use the "b"
prefix and conceptually it might make more sense to only use it for byte
arrays. I can go through and rename the ones are that currently using it so
that they're not if you think that'd improve things.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
