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

Removed ioredis/ioredis-mock, added Redis from Satellite instead #2615

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Removed ioredis/ioredis-mock, added Redis from Satellite instead #2615

merged 1 commit into from
Jan 18, 2022

Conversation

TueeNguyen
Copy link
Contributor

@TueeNguyen TueeNguyen commented Dec 15, 2021 •
edited
Loading

Issue This PR Addresses

This fixes #2574

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

  • Changed backend/lib/redis.js to use Redis from satellite instead of ioredis/ioredis-mock
  • Removed ioredis/ioredis-mock from package.json

How was this tested?

  • All docker containers ran normally using pnpm services:start
  • The local telescope instance ran normally using pnpm start

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@gitpod-io
Copy link

gitpod-io bot commented Dec 15, 2021

@humphd humphd requested a review from HyperTHD December 15, 2021 22:21
@Qiwen-Yu
Copy link
Contributor

This PR is to change to use Redis from satellite instead of using ioredis/ioredis-mock. This change works.

Qiwen-Yu
Qiwen-Yu reviewed Dec 15, 2021
View reviewed changes
Copy link
Contributor

@Qiwen-Yu Qiwen-Yu left a comment

Choose a reason for hiding this comment

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

This PR is to change to use Redis from satellite instead of using ioredis/ioredis-mock. This change works.

Qiwen-Yu
Qiwen-Yu reviewed Dec 15, 2021
View reviewed changes
Copy link
Contributor

@Qiwen-Yu Qiwen-Yu left a comment

Choose a reason for hiding this comment

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

It looks good to me

@TueeNguyen
Copy link
Contributor Author

This PR is to change to use Redis from satellite instead of using ioredis/ioredis-mock. This change works.

Do you want me to change the PR name to be clearer?

@Qiwen-Yu
Copy link
Contributor

This PR is to change to use Redis from satellite instead of using ioredis/ioredis-mock. This change works.

Do you want me to change the PR name to be clearer?

I think it's good.

HyperTHD
HyperTHD requested changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@HyperTHD HyperTHD left a comment

Choose a reason for hiding this comment

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

Looks good, just a small tweak and it'll be good

src/backend/lib/redis.js Show resolved Hide resolved
humphd
humphd previously approved these changes Dec 16, 2021
View reviewed changes
@HyperTHD HyperTHD self-requested a review December 16, 2021 23:06
HyperTHD
HyperTHD previously approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@HyperTHD HyperTHD left a comment

Choose a reason for hiding this comment

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

Read the comments, just remember to rebase and pick up changes!

@TueeNguyen TueeNguyen dismissed stale reviews from HyperTHD and humphd via 31d2698 January 10, 2022 00:08
humphd
humphd requested changes Jan 10, 2022
View reviewed changes
Copy link
Contributor

@humphd humphd 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 changing way more than it should be. Can you rebase to get the pinned pnpm versions?

@TueeNguyen
Copy link
Contributor Author

This is changing way more than it should be. Can you rebase to get the pinned pnpm versions?

Right, I notice that it changes a lot, I'll try rebasing again to get the pinned versions

humphd
humphd reviewed Jan 10, 2022
View reviewed changes
package.json Outdated Show resolved Hide resolved
@humphd humphd added this to the 2.5 Release milestone Jan 13, 2022
@TueeNguyen TueeNguyen mentioned this pull request Jan 18, 2022
Merged
4 tasks
@TueeNguyen
Copy link
Contributor Author

Use satellite ^1.x as mentioned in #2664

@TueeNguyen TueeNguyen requested a review from DukeManh January 18, 2022 19:05
DukeManh
DukeManh approved these changes Jan 18, 2022
View reviewed changes
pnpm-lock.yaml Show resolved Hide resolved
@DukeManh DukeManh self-requested a review January 18, 2022 19:41
@TueeNguyen TueeNguyen merged commit 0ddaa15 into Seneca-CDOT:master Jan 18, 2022
TDDR added a commit to TDDR/telescope that referenced this pull request Jan 19, 2022
@TDDR
Revert "Removed ioredis/ioredis-mock, added Redis from Satellite inst…
a8c881a 
…ead (Seneca-CDOT#2615)"

This reverts commit 0ddaa15.
@TDDR TDDR mentioned this pull request Jan 19, 2022
Merged
@TueeNguyen TueeNguyen mentioned this pull request Jan 20, 2022
Closed
DukeManh pushed a commit that referenced this pull request Jan 20, 2022
@TDDR @DukeManh
Revert "Removed ioredis/ioredis-mock, added Redis from Satellite inst…
dece55b 
…ead (#2615)"

This reverts commit 0ddaa15.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Qiwen-Yu Qiwen-Yu Qiwen-Yu left review comments

@humphd humphd humphd approved these changes

@DukeManh DukeManh DukeManh approved these changes

@HyperTHD HyperTHD HyperTHD left review comments

Assignees

@TueeNguyen TueeNguyen

Labels
None yet
Projects
None yet
Milestone
2.5 Release
Development

Successfully merging this pull request may close these issues.

Use Redis from Satellite in all microservices that need them
5 participants
@TueeNguyen @Qiwen-Yu @humphd @HyperTHD @DukeManh