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

feat(pubsub): Add shared context among Redis instances #1110

Merged
merged 2 commits into from
Jan 22, 2022
Merged

feat(pubsub): Add shared context among Redis instances #1110

merged 2 commits into from
Jan 22, 2022

Conversation

fersilva16
Copy link

@fersilva16 fersilva16 commented Jan 12, 2022

I'm continuing the #997.

  • Create a shared expires instance for context
  • Create a shared data instance for context

Closes #997 and #773

@fersilva16 fersilva16 changed the title Add shared context among Redis instances fix(pubsub): Add shared context among Redis instances Jan 13, 2022
@fersilva16 fersilva16 marked this pull request as ready for review January 13, 2022 13:08
Copy link
Owner

@stipsan stipsan left a comment

Choose a reason for hiding this comment

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

This is fantastic! Let's roll it out! 🥳

@stipsan
Copy link
Owner

stipsan commented Jan 20, 2022

I'll release this as soon as the tests pass, and change the commit to use feat instead of fix as I consider this new functionality ☺️

@fersilva16 fersilva16 changed the title fix(pubsub): Add shared context among Redis instances feat(pubsub): Add shared context among Redis instances Jan 20, 2022
@stipsan
Copy link
Owner

stipsan commented Jan 21, 2022

Just did a local checkout to run the test suite, and I'm getting the same failing tests as https://github.com/stipsan/ioredis-mock/runs/4892890568?check_suite_focus=true#step:4:11167

@fersilva16 are you able to run through the test suite?

@fersilva16
Copy link
Author

are you able to run through the test suite?

No, my tests are failing too, I'm implemented the shared expires/data and it fixes almost all broken tests, but I'm still working on the rest of the tests, some of them need a refactor.

@stipsan
Copy link
Owner

stipsan commented Jan 21, 2022

are you able to run through the test suite?

No, my tests are failing too, I'm implemented the shared expires/data and it fixes almost all broken tests, but I'm still working on the rest of the tests, some of them need a refactor.

Alright, let us know if there's anything we can do to help ☺️

@fersilva16
Copy link
Author

Now the tests should pass. And should I rebase the commits to one commit only?

@stipsan
Copy link
Owner

stipsan commented Jan 21, 2022

Now the tests should pass. And should I rebase the commits to one commit only?

Awesome! 🥳 Rebase or not is entirely up to you, as all PRs are squash & merged anyways 😌

@fersilva16
Copy link
Author

I think it's done now! 🥳🎉

@stipsan
Copy link
Owner

stipsan commented Jan 21, 2022

I think it's done now! 🥳🎉

Thank you so much @fersilva16! 🥳 this is fantastic, and I know a lot of people have been waiting for this! (Especially @sibelius) 😄

@stipsan
Copy link
Owner

stipsan commented Jan 21, 2022

The error on the CI seems to be related to an error I saw on an another PR earlier today: #1106 (comment)

@sibelius
Copy link
Contributor

it is happening

@fersilva16
Copy link
Author

Fixed!

@stipsan stipsan merged commit 3da06de into stipsan:master Jan 22, 2022
@stipsan
Copy link
Owner

stipsan commented Jan 22, 2022

🎉 This PR is included in version 5.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants