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.

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

Added memcached.sess_prefix for --SERVER= connection string #282

Closed
wants to merge 1 commit into from

Conversation

dyeldandi
Copy link
Contributor

A quick fix for issue #281

@sodabrew
Copy link
Contributor

It looks like this fix only applies the prefix if --SERVERS is not used. It looks like both cases need to be handled (or refactored so that the same code adds prefixes regardless of how the servers array was specified).

@sodabrew sodabrew added this to the 3.0.1 milestone Jan 24, 2017
@dyeldandi
Copy link
Contributor Author

dyeldandi commented Jan 24, 2017

It looks like this fix only applies the prefix if --SERVERS is not used.

Not exactly. It jumps to that "else" branch on "goto success;" It's kind of ugly, but it was there initially and I tried to do the minimal changes keeping the logic as it is.

@sodabrew
Copy link
Contributor

Could I ask you to rebase your branch against master? There are some merge conflicts right now.

@dyeldandi
Copy link
Contributor Author

You don't need to merge this to master, it looks like in php7 branch this part has been completely rewritten.

@sodabrew
Copy link
Contributor

Im probably not going to make another 2.x release, but if you wanted this fix for 2.2.1, adjust the PR base branch to REL2_0

@dyeldandi
Copy link
Contributor Author

Yes, that would be very nice. Created pull #304. Thanks a lot!

sodabrew added a commit that referenced this pull request Jan 25, 2017
REL2_0 pull request, replacement for pull requests #282 and #165
@sodabrew sodabrew closed this Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants