Skip to content

Remove gopher protocol support. #9057

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

Merged
merged 2 commits into from
Jun 16, 2021
Merged

Conversation

yoav-steinberg
Copy link
Contributor

@yoav-steinberg yoav-steinberg commented Jun 8, 2021

Gopher support was added mainly because it was simple (trivial to add).
But apparently even something that was trivial at the time, does cause complications
down the line when adding more features.
We recently ran into a few issues with io-threads conflicting with the gopher support.
We had to either complicate the code further in order to solve them, or drop gopher.
AFAIK it's completely unused, so we wanna chuck it, rather than keep supporting it.

See #8989

@yoav-steinberg yoav-steinberg added the approval-needed Waiting for core team approval to be merged label Jun 8, 2021
@yoav-steinberg yoav-steinberg requested a review from oranagra June 8, 2021 14:56
@oranagra oranagra changed the title Remove gopher. See #8989. Remove gopher protocol support. Jun 9, 2021
@oranagra oranagra added release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus labels Jun 9, 2021
Copy link
Member

@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.

@yoav-steinberg i added a top comment (to be used as commit comment), please review it and edit if necessary.

@redis/core-team please approve.

@yoav-steinberg
Copy link
Contributor Author

@oranagra The top comment is good. Fixed some typos. You can merge.

@oranagra oranagra merged commit 362786c into redis:unstable Jun 16, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
Gopher support was added mainly because it was simple (trivial to add).
But apparently even something that was trivial at the time, does cause complications
down the line when adding more features.
We recently ran into a few issues with io-threads conflicting with the gopher support.
We had to either complicate the code further in order to solve them, or drop gopher.
AFAIK it's completely unused, so we wanna chuck it, rather than keep supporting it.
@oranagra oranagra added the breaking-change This change can potentially break existing application label Jan 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval-needed Waiting for core team approval to be merged breaking-change This change can potentially break existing application release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants