Skip to content
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

Implements BITFIELD_RO command #2174

Merged
merged 4 commits into from
Apr 13, 2020

Conversation

yangbodong22011
Copy link
Collaborator

In the redis unstable branch, the bitfield_ro command is added, refer: this .

Copy link
Collaborator

@sazzad16 sazzad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there's a new read sub-command for BITFIELD command? So I don't think bitfieldReadonly(T key, T type, T offset) is a good idea. bitfield(T key, T... arguments) bitfieldReadonly(T key, T... arguments) may look ugly, but it's a much safer option.

@yangbodong22011
Copy link
Collaborator Author

What if there's a new read sub-command for BITFIELD command?

Can you give a specific code example?

@sazzad16
Copy link
Collaborator

Sorry, there was a typo in my previous comment.

About example, in JedisCommands interface, instead of

List<Long> bitfieldReadonly(String key, String type, String offset);

my suggestion is to use

List<Long> bitfieldReadonly(String key, String... arguments);

@yangbodong22011
Copy link
Collaborator Author

List bitfieldReadonly(String key, String... arguments);

Is this for scalability? Currently bitfield_ro only supports the GET subcommand, So i hardcoded it.

public void bitfieldReadonly(final byte[] key, final byte[] type, final byte[] offset) {
    sendCommand(BITFIELD_RO, key, SafeEncoder.encode("GET"), type, offset);
}

For scalability reasons, I think your suggestion is good.

@sazzad16
Copy link
Collaborator

sazzad16 commented Apr 1, 2020

Yes.

@yangbodong22011
Copy link
Collaborator Author

@sazzad16 Thanks,updated.

@sazzad16 sazzad16 added this to the 3.4.0 milestone Apr 1, 2020
@sazzad16 sazzad16 requested a review from gkorland April 1, 2020 07:04
@gkorland gkorland merged commit ab36b36 into redis:master Apr 13, 2020
@sazzad16 sazzad16 modified the milestones: 3.4.0, 3.3.0 Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants