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

Format after #1988 and #1968 #2078

Closed
wants to merge 6 commits into from
Closed

Conversation

sazzad16
Copy link
Collaborator

@sazzad16 sazzad16 commented Oct 16, 2019

@sazzad16 sazzad16 added this to the 3.2.0 milestone Oct 16, 2019
@sazzad16 sazzad16 self-assigned this Oct 16, 2019
@sazzad16 sazzad16 changed the title Modify #1988 according to #1968 Fix and further modify #1988 Oct 28, 2019
@gkorland
Copy link
Contributor

gkorland commented Dec 9, 2019

@sazzad16 it is really hard to see what is #2078 is adding on top of #1968

@sazzad16
Copy link
Collaborator Author

sazzad16 commented Dec 9, 2019

@gkorland

  1. Almost all of Jedis methods that takes a count param, receives it as an int. There are only two exceptions in Jedis class. As zpopmin is yet to release, I changed its count to int.

  2. ZPOPMIN without count returns only one Tuple (or none). Instead of returning a Set<Tuple> with only one element, it'd be better to return Tuple. ZPOPMAX implemented zpopmax, zpopmax with count #1968 is changed to be like that. I included this change in this PR.

@sazzad16 sazzad16 changed the title Fix and further modify #1988 Format after #1988 and #1968 Dec 9, 2019
@sazzad16
Copy link
Collaborator Author

sazzad16 commented Dec 9, 2019

@gkorland I forgot that I had changed the same changes added most of the changes of this PR also in 915a810 which are merged through #1968 just now. As a result, there are not much left to see separately.

@gkorland
Copy link
Contributor

gkorland commented Dec 9, 2019

So can we close this PR?

@gkorland
Copy link
Contributor

gkorland commented Dec 9, 2019

or at lease undo all the none relevant changes?

Copy link
Contributor

@gkorland gkorland left a comment

Choose a reason for hiding this comment

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

please undo uneeded changes

src/main/java/redis/clients/jedis/PipelineBase.java Outdated Show resolved Hide resolved
src/main/java/redis/clients/jedis/PipelineBase.java Outdated Show resolved Hide resolved
src/main/java/redis/clients/jedis/BuilderFactory.java Outdated Show resolved Hide resolved
src/main/java/redis/clients/jedis/BuilderFactory.java Outdated Show resolved Hide resolved
src/main/java/redis/clients/jedis/BuilderFactory.java Outdated Show resolved Hide resolved
@sazzad16
Copy link
Collaborator Author

sazzad16 commented Dec 9, 2019

@gkorland

please undo uneeded changes

done!

@gkorland
Copy link
Contributor

@sazzad16 is still seems like a lot of code just moved around

@sazzad16 sazzad16 closed this Dec 10, 2019
@sazzad16 sazzad16 removed this from the 3.2.0 milestone Dec 10, 2019
@sazzad16 sazzad16 deleted the 1988-by-1968 branch March 11, 2020 10:53
@sazzad16 sazzad16 restored the 1988-by-1968 branch March 11, 2020 10:53
@sazzad16 sazzad16 deleted the 1988-by-1968 branch December 26, 2020 14:31
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

2 participants