New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java: Add GETBIT
command
#1400
Java: Add GETBIT
command
#1400
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor doc comments, but otherwise LGTM.
java/client/src/main/java/glide/api/models/BaseTransaction.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/models/BaseTransaction.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/models/BaseTransaction.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of comments
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/models/BaseTransaction.java
Outdated
Show resolved
Hide resolved
* assert payload == 1L; // The second bit for string stored at "myKey1" is set to 1. | ||
* }</pre> | ||
*/ | ||
CompletableFuture<Long> getbit(String key, long offset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider something smaller than Long for the return type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we only had Long and Double types for all number types. Would introducing Integer/Byte/Short cause any issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldnt it be a boolean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets stick with long until we have a consensus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per the discussion offline, we are going to use long. This can be switched in the future if a boolean or byte is preferred.
4fdc477
to
2129dda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good job :)
* assert payload == 1L; // The second bit for string stored at "myKey1" is set to 1. | ||
* }</pre> | ||
*/ | ||
CompletableFuture<Long> getbit(String key, long offset); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldnt it be a boolean?
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/models/BaseTransaction.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/commands/BitmapBaseCommands.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/api/models/BaseTransaction.java
Outdated
Show resolved
Hide resolved
2129dda
to
fc9bb15
Compare
* Added GETBIT command Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Applied spotless Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Removed duplicate test case Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> * Added comments to tests Signed-off-by: Guian Gumpac <guian.gumpac@improving.com> --------- Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>
Co-authored-by: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com>
Co-authored-by: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com>
Co-authored-by: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com>
…s.java Co-authored-by: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com>
…s.java Co-authored-by: jonathanl-bq <72158117+jonathanl-bq@users.noreply.github.com>
466ac62
to
2c46236
Compare
Issue #, if available:
N/A
Description of changes:
Implements
GETBIT
command in the java client.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.