Skip to content

Update creating-redis-service-containers.md #22662

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
Merged
hubwriter merged 7 commits into from
Sep 15, 2023
Merged

Update creating-redis-service-containers.md #22662

hubwriter merged 7 commits into from
Sep 15, 2023

Conversation

@ChoHadam
Copy link
Contributor

@ChoHadam ChoHadam commented Dec 13, 2022 •
edited
Loading

Why:

Closes #22661

What's being changed (if available, include any code snippets, screenshots, or gifs):

I wanted the client.js code to work with the current version(4.x) of redis npm module. So I modified it by the following steps.

Step1
Modify client.js code to operate while maintaining v3 syntax in v4 using legacy mode.
Step2
Fix some comments in client.js code.
Step 3
Modify client.js code to promise-based async code using ES6 and disable legacy mode.

Before creating this pull request, I followed the guide and created a Redis client in my workflow to make sure that the modified code works. You can see the results in the following Redis runner example.

Redis runner example
스크린샷 2022-12-13 오후 5 49 21

Check off the following:

  • I have reviewed my changes in staging (look for the "Automatically generated comment" and click the links in the "Preview" column to view your latest changes).
  • For content changes, I have completed the self-review checklist.

@welcome
Copy link

welcome bot commented Dec 13, 2022

Thanks for opening this pull request! A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label Dec 13, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 13, 2022 •
edited
Loading

Automatically generated comment ℹ️

This comment is automatically generated and will be overwritten every time changes are committed to this branch.

The table contains an overview of files in the content directory that have been changed in this pull request. It's provided to make it easy to review your changes on the staging site. Please note that changes to the data directory will not show up in this table.

Content directory changes

You may find it useful to copy this table into the pull request summary. There you can edit it to share links to important articles or changes and to give a high-level overview of how the changes in your pull request support the overall goals of the pull request.

Source Preview Production What Changed
actions/using-containerized-services/creating-redis-service-containers.md fpt
ghec
ghes@ 3.10 3.9 3.8 3.7 3.6
ghae
 fpt
ghec
ghes@ 3.10 3.9 3.8 3.7 3.6
ghae

fpt: Free, Pro, Team
ghec: GitHub Enterprise Cloud
ghes: GitHub Enterprise Server
ghae: GitHub AE

@cmwilson21
Copy link
Contributor

cmwilson21 commented Dec 13, 2022

@ChoHadam Thank you for opening a PR and linking it to your issue! ✨

I'll get this triaged for review! ⚡

@cmwilson21 cmwilson21 added content This issue or pull request belongs to the Docs Content team actions This issue or pull request should be reviewed by the docs actions team waiting for review Issue/PR is waiting for a writer's review and removed triage Do not begin working on this issue until triaged by the team labels Dec 13, 2022
@lucascosti lucascosti added the needs SME This proposal needs review from a subject matter expert label Jan 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2023

Thanks for opening a pull request! We've triaged this issue for technical review by a subject matter expert 👀

@lilmac82

This comment was marked as spam.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented Feb 15, 2023

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Feb 15, 2023
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Feb 16, 2023
n6wbb6

This comment was marked as spam.

Sign in to view

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2023

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Apr 28, 2023
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Jun 19, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 17, 2023

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Jul 17, 2023
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Jul 18, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 15, 2023

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Aug 15, 2023
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Aug 16, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2023

This is a gentle bump for the docs team that this PR is waiting for technical review.

@github-actions github-actions bot added the SME stale The request for an SME has staled label Sep 13, 2023
@cmwilson21 cmwilson21 removed the SME stale The request for an SME has staled label Sep 13, 2023
hubwriter
hubwriter approved these changes Sep 15, 2023
View reviewed changes
Copy link
Contributor

@hubwriter hubwriter left a comment

Choose a reason for hiding this comment

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

This looks like a good update to me. 👍
Many thanks for doing this @ChoHadam.
Apologies it has taken so long to get this reviewed.

@hubwriter hubwriter added this pull request to the merge queue Sep 15, 2023
Merged via the queue into github:main with commit 2d1c27a Sep 15, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 15, 2023

Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you're looking for your next contribution, check out our help wanted issues ⚡

@ChoHadam
Copy link
Contributor Author

ChoHadam commented Nov 15, 2023 •
edited
Loading

Hi @hubwriter , I'm hadam.👋🏻 I'm one of the favorite readers of github docs.
Thank you for merging this PR! I have some questions about your review.

Can you please let me know why it took so long for this PR to get reviewed?
I just wonder if there wasn't enough time to review, or if there were difficulties understanding my PR (e.g. poor readability, lack of my English skills).

And I'm also curious why my PR was able to be merged all at once without much feedback.

This is my first open source contribution, so I think your comments will help me make better contributions in the future. 😊

@eissa11
Copy link

eissa11 commented Nov 15, 2023 via email

@hubwriter
Copy link
Contributor

hubwriter commented Nov 15, 2023

Hi there @ChoHadam 👋

Apologies again that this PR went un-reviewed for so long. We usually review open-source contributions a lot quicker. Unfortunately, this one sat in our review backlog for a long time. There was no particular reason for this - just that we're a little under-staffed in the internal Docs team that reviews all PRs in this repo.

Because you provided great information about the reason for the change, in the issue you raised, and evidence of testing your work, I was able to merge this PR without asking for further work. This doesn't always happen but I was glad that, after leaving this in the backlog for so long, I was able just to go ahead and merge it.

I hope this doesn't put you off making further contributions to the GitHub docs in future. We really appreciate your work helping to keep the docs up to date. 🙇‍♂️

@ChoHadam
Copy link
Contributor Author

ChoHadam commented Nov 16, 2023

Thank you for your reply @hubwriter .
It was nice talking with you. It means a lot to me 😊
Have a nice day ☀️

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

Reviewers

@hubwriter hubwriter hubwriter approved these changes

+1 more reviewer

@n6wbb6 n6wbb6 n6wbb6 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

actions This issue or pull request should be reviewed by the docs actions team content This issue or pull request belongs to the Docs Content team needs SME This proposal needs review from a subject matter expert waiting for review Issue/PR is waiting for a writer's review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Client.js does not match the current version of redis npm module

7 participants

@ChoHadam @cmwilson21 @lilmac82 @eissa11 @hubwriter @n6wbb6 @lucascosti