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.

Sign up for GitHub

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

Jump to bottom

Implements objecthelp and objectFREQ method. #2141

Merged
merged 19 commits into from
Apr 13, 2020
Merged

Implements objecthelp and objectFREQ method. #2141

merged 19 commits into from
Apr 13, 2020

Conversation

dengliming
Copy link
Contributor

@dengliming dengliming commented Feb 15, 2020 •
edited
Loading

A PR for #2116 and #2117

This was referenced Feb 15, 2020
Closed
Closed
@dengliming dengliming requested a review from gkorland February 15, 2020 20:27
@dengliming dengliming requested a review from sazzad16 February 16, 2020 05:51
@dengliming
Copy link
Contributor Author

@sazzad16 Travis ci build fail. Can you rerun it?

@dengliming dengliming requested a review from sazzad16 February 16, 2020 08:10
@sazzad16
Copy link
Collaborator

@dengliming Apparently Travis CI removed rerun option or limited its support. I couldn't find it, sorry!

@dengliming
Copy link
Contributor Author

The command "make travis-install" failed and exited. Do you have any idea for this? @sazzad16

@dengliming
Copy link
Contributor Author

@dengliming Apparently Travis CI removed rerun option or limited its support. I couldn't find it, sorry!

You should Log in to Travis to check (I think)

@sazzad16
Copy link
Collaborator

@dengliming

You should Log in to Travis to check (I think)

I did.

This seems to be due to an unrelated issue. You may after a day or two.

@gkorland ping!

@gkorland
Copy link
Contributor

I reran travis and it seems to pass now, it seems like they had some network issues

@dengliming
Copy link
Contributor Author

@sazzad16 @gkorland Can you review these changes?I also want to implement other feature from this repository recently. Thanks.

sazzad16
sazzad16 requested changes Feb 21, 2020
View reviewed changes
src/main/java/redis/clients/jedis/commands/BinaryRedisPipeline.java Outdated Show resolved Hide resolved
src/test/java/redis/clients/jedis/tests/commands/ObjectCommandsTest.java Outdated Show resolved Hide resolved
src/test/java/redis/clients/jedis/tests/commands/ObjectCommandsTest.java Outdated Show resolved Hide resolved
src/test/java/redis/clients/jedis/tests/commands/ObjectCommandsTest.java Outdated Show resolved Hide resolved
src/test/java/redis/clients/jedis/tests/commands/ObjectCommandsTest.java Outdated
Comment on lines 37 to 38
jedis.disconnect();
jedis1.disconnect();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Follow setUp().

@dengliming
Copy link
Contributor Author

@sazzad16 It was done.

sazzad16
sazzad16 requested changes Feb 21, 2020
View reviewed changes
src/test/java/redis/clients/jedis/tests/commands/JedisCommandTestBase.java Outdated Show resolved Hide resolved
src/test/java/redis/clients/jedis/tests/commands/JedisCommandTestBase.java Outdated Show resolved Hide resolved
src/test/java/redis/clients/jedis/tests/commands/ObjectCommandsTest.java Outdated
Comment on lines 36 to 37
super.tearDown();
lfuJedis.disconnect();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reverse order.

Makefile Outdated Show resolved Hide resolved
src/main/java/redis/clients/jedis/commands/RedisPipeline.java Outdated Show resolved Hide resolved
@dengliming
Copy link
Contributor Author

@sazzad16 Thanks again for your patience.

@dengliming dengliming requested a review from sazzad16 February 21, 2020 15:04
sazzad16
sazzad16 previously approved these changes Feb 21, 2020
View reviewed changes
@sazzad16 sazzad16 self-requested a review February 21, 2020 15:10
@sazzad16 sazzad16 dismissed their stale review February 21, 2020 15:13

One change request was shown as resolved, even though it wasn't. So I missed it.

@sazzad16 sazzad16 added this to the 3.3.0 milestone Mar 16, 2020
@sazzad16 sazzad16 modified the milestones: 3.3.0, 3.4.0 Mar 16, 2020
@dengliming
Copy link
Contributor Author

Any progress on this? @sazzad16

@startjava
Copy link

startjava commented Mar 20, 2020 via email

@sazzad16
Copy link
Collaborator

@dengliming We'll need review from other collaborator. So please wait. Thanks!

@gkorland gkorland merged commit 8135738 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
Reviewers

@sazzad16 sazzad16 sazzad16 approved these changes

@gkorland gkorland gkorland approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@dengliming @sazzad16 @gkorland @startjava