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

Impl zpopmin zpopmax #1986

Closed
wants to merge 4 commits into from

Conversation

kommradHomer
Copy link
Contributor

I have implemented 2 commands for the Sorted Sets . ZPOPMIN and ZPOPMAX

ZPOPMIN fixed after testing
implemented ZPOPMAX . created test for ZPOPMAX
@marcosnils
Copy link
Contributor

marcosnils commented Jun 1, 2019

Thx for contributing to Jedis. Can you please add this commands in Pipeline and Cluster? Here's an example of another PR which added those implementations as well.

https://github.com/xetorthio/jedis/pull/1841/files

@kommradHomer
Copy link
Contributor Author

Hello , I have added what i thought is missing and committed , after your warning. Do i need to open another pull request ? This is the first time i'm using this mechanism . Thank you

@sazzad16
Copy link
Collaborator

sazzad16 commented Jun 4, 2019

@marcosnils We already have a pending PR for zpopmax at #1968

@kommradHomer
Copy link
Contributor Author

@sazzad16 how about zpopmin ? should i cancel or close this PR ?

@marcosnils
Copy link
Contributor

@kommradHomer yup, zpopmin is still needed. You can go ahead and send just that one.

Apologies for the confusin.

@sazzad16
Copy link
Collaborator

@kommradHomer Thanks :)

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.

None yet

3 participants