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

redis-cli TLS connections #2635

Merged
merged 3 commits into from
Jul 10, 2023
Merged

redis-cli TLS connections #2635

merged 3 commits into from
Jul 10, 2023

Conversation

rrelledge
Copy link
Collaborator

@rrelledge rrelledge self-assigned this Jun 20, 2023
yoavredis
yoavredis previously requested changes Jun 21, 2023
View reviewed changes
Copy link
Collaborator

@yoavredis yoavredis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks OK to me.
But I think it should also be reviewed and approved by those who maintain redis-cli.
I suggest to ask @oranagra to take a look

@rrelledge rrelledge requested a review from oranagra June 21, 2023 10:22
oranagra
oranagra reviewed Jun 21, 2023
View reviewed changes
Copy link
Contributor

@oranagra oranagra left a comment

Choose a reason for hiding this comment

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

i've never connected with redis-cli to RE using TLS, so i can't comment on most of that text.
i suggest asking a proxy developer or QA person that tested it.

content/rs/references/cli-utilities/redis-cli/_index.md Outdated Show resolved Hide resolved
kaitlynmichael
kaitlynmichael approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@kaitlynmichael kaitlynmichael left a comment

Choose a reason for hiding this comment

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

One wording suggestion

content/rs/references/cli-utilities/redis-cli/_index.md Outdated Show resolved Hide resolved
rrelledge and others added 2 commits June 26, 2023 14:40
@rrelledge @kaitlynmichael
Apply suggestions from code review
fca8bb2 
Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
@rrelledge
Feedback updates
f44d79f
@rrelledge rrelledge requested a review from yoavredis July 10, 2023 19:31
@rrelledge rrelledge dismissed yoavredis’s stale review July 10, 2023 19:36

Received a review from Oran, and Kurt has also taken a look at the TLS connection section and provided feedback.

@rrelledge rrelledge merged commit 0eae06b into master Jul 10, 2023
4 checks passed
@rrelledge rrelledge deleted the DOC-1760 branch July 10, 2023 19:40
kaitlynmichael added a commit that referenced this pull request Jul 13, 2023
@rrelledge @kaitlynmichael
redis-cli TLS connections (#2635)
236173d 
* redis-cli TLS connections

* Apply suggestions from code review

Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>

* Feedback updates

---------

Co-authored-by: Kaitlyn Michael <76962844+kaitlynmichael@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oranagra oranagra oranagra left review comments

@kaitlynmichael kaitlynmichael kaitlynmichael approved these changes

@dwdougherty dwdougherty Awaiting requested review from dwdougherty

@cmilesb cmilesb Awaiting requested review from cmilesb

@yoavredis yoavredis Awaiting requested review from yoavredis

Assignees

@rrelledge rrelledge

Labels
p_Cloud p_Software
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rrelledge @oranagra @kaitlynmichael @yoavredis