Skip to content

Remove gopher protocol support. - #9057

Merged
oranagra merged 2 commits into
redis:unstablefrom
yoav-steinberg:remove_gopher
Jun 16, 2021
Merged

oranagra merged 2 commits into
redis:unstablefrom
yoav-steinberg:remove_gopher

Conversation

@yoav-steinberg

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

Copy link
Copy Markdown
Contributor

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

@oranagra oranagra left a comment

Copy link
Copy Markdown
Member

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.

Comment thread src/config.c
@yoav-steinberg

Copy link
Copy Markdown
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