Skip to content

Conversation

@vietqhoang
Copy link
Contributor

@vietqhoang vietqhoang commented Oct 30, 2025

@vietqhoang
Copy link
Contributor Author

Please let me know if there is any improvement to the PR I can do in order to get this merged in.

@redis redis deleted a comment from jit-ci bot Oct 30, 2025
assert_equal [1], r.hexpire("foo", 1, "f1")
assert r.hexists("foo", "f1")

sleep(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sleeping in test suite is always a bad idea.

A better way to test hexpire is to implement httl, and use it to check the TTL changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I've made the changes to introduce httl and used it within the hexpire test. See 3662a56

vietqhoang added a commit to vietqhoang/redis-rb that referenced this pull request Oct 30, 2025
@byroot
Copy link
Collaborator

byroot commented Oct 30, 2025

@vietqhoang I was editing your PR, please stop pushing in your branch.

@byroot byroot merged commit 83496ac into redis:master Oct 30, 2025
22 of 28 checks passed
@vietqhoang
Copy link
Contributor Author

Sorry, didn't catch you were also editing.

Just want to note the change added to distributed

https://github.com/vietqhoang/redis-rb/blob/2c4eaac1c1ef07a6b33bfe2ac4d1945912b09c8c/lib/redis/distributed.rb#L925-L927

The ttl argument is unnecessary since httl doesn't intake ttl. Tests still passed...

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.

2 participants