Skip to content

Add redis v7's ExpireAtNX, ExpireAtXX, ExpireAtGT, ExpireAtLT, PExpireNX, PExpireXX, PExpireGT, PExpireLT, PExpireAtNX, PExpireAtXX, PExpireAtGT, PExpireAtLT #2589

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

carner
Copy link
Contributor

@carner carner commented May 11, 2023

Hi ^^

Small commit to add ExpireAtNX, ExpireAtXX, ExpireAtGT, ExpireAtLT of Redis v7
doc: https://redis.io/commands/expireat
Small commit to add PExpireNX, PExpireXX, PExpireGT, PExpireLT of Redis v7
doc: https://redis.io/commands/pexpire
Small commit to add PExpireAtNX, PExpireAtXX, PExpireAtGT, PExpireAtLT of Redis v7
doc: https://redis.io/commands/pexpireat

…eNX, PExpireXX, PExpireGT, PExpireLT, PExpireAtNX, PExpireAtXX, PExpireAtGT, PExpireAtLT

feat: Add redis v7's NX, XX, GT, LT expireat, pexpire, pexpireat variants
Copy link
Contributor

@SoulPancake SoulPancake left a comment

Choose a reason for hiding this comment

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

Do you plan to add tests? as we need to add coverage for the new commands. @carner

@carner
Copy link
Contributor Author

carner commented May 15, 2023

ok, I will do it.

add tests to coverage for the new commands
@chayim chayim added the feature label May 15, 2023
@chayim chayim requested a review from elena-kolevska May 15, 2023 07:54
@carner
Copy link
Contributor Author

carner commented May 15, 2023

the tests added. @chayim @SoulPancake

Copy link
Contributor

@SoulPancake SoulPancake left a comment

Choose a reason for hiding this comment

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

LGTM

@elena-kolevska
Copy link
Contributor

Thank you very much for your contribution @carner :)
I left a few comments, mostly small things that should be easy to fix. Let me know if you need help with any of them.

@carner
Copy link
Contributor Author

carner commented May 16, 2023

thank you

@carner
Copy link
Contributor Author

carner commented Jun 9, 2023

@elena-kolevska please review code.

@elena-kolevska
Copy link
Contributor

elena-kolevska commented Jun 16, 2023

Hi @carner . Can you please check the box “Allow edits from maintainers.”? I would like to push a commit that cleans up some details, and we can merge this PR.

@carner
Copy link
Contributor Author

carner commented Jun 19, 2023

the box is checked。 @elena-kolevska
image

elena-kolevska
elena-kolevska previously approved these changes Jun 19, 2023
@elena-kolevska
Copy link
Contributor

@monkey92t should we merge?

@ndyakov
Copy link
Member

ndyakov commented Feb 4, 2025

@carner Seems this PR was lost a while back. Can you sync with master and see if we can work on a final review of it?

@ndyakov
Copy link
Member

ndyakov commented Mar 18, 2025

Let’s see if we can fix the code. @carner let me know if you need any assistance.

@carner
Copy link
Contributor Author

carner commented Mar 18, 2025

Let’s see if we can fix the code. @carner let me know if you need any assistance.让我们看看是否可以修复代码。如果您需要任何帮助,@carner告诉我。

@ndyakov Finish.

@ndyakov
Copy link
Member

ndyakov commented Mar 24, 2025

@carner after a closer look, we do use another convention of passing options to commands to not blow up the function list by adding a separate function per argument. Check the FTSearchWithArgs for example. You can also look at #3305 for inspiration.

@ndyakov
Copy link
Member

ndyakov commented Jun 5, 2025

Hello @carner , do you plan to work on this? I can take over if you don't have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 participants