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

Conversation

yoav-steinberg
Copy link
Contributor

@yoav-steinbergyoav-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-steinbergyoav-steinberg added the approval-needed Waiting for core team approval to be merged label Jun 8, 2021
@oranagraoranagra changed the title Remove gopher. See #8989.Remove gopher protocol support.Jun 9, 2021
@oranagraoranagra 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

@oranagraoranagra 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
ContributorAuthor

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

@oranagraoranagra merged commit 362786c into redis:unstableJun 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.
@oranagraoranagra added the breaking-change This change can potentially break existing application label Jan 23, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval-neededWaiting for core team approval to be mergedbreaking-changeThis change can potentially break existing applicationrelease-notesindication that this issue needs to be mentioned in the release notesstate:major-decisionRequires core team consensus
4 participants
@yoav-steinberg@yossigo@oranagra@madolson
close