8000 Add Authentication Support by Evanraisul · Pull Request #182 · bradfitz/gomemcache · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Authentication Support #182

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
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add Authentication Support #182

wants to merge 5 commits into from

Conversation

Evanraisul
Copy link
@Evanraisul Evanraisul commented Sep 30, 2024 •
edited
Loading

Signed-off-by: Muhammad Raisul Islam Evan evanraisul@gmail.com

@dormando
Copy link
Collaborator
dormando commented Apr 4, 2025

@Evanraisul do you want this change or can we close it? You've imported and vendored a ton of stuff that's unnecessary for the change itself.

The PR should be focused on just adding the feature you want.

Thanks!

@Evanraisul
Copy link
Author
Evanraisul commented Apr 4, 2025 •
edited
Loading

I have updated the PR. You can review it now.

@Evanraisul Evanraisul closed this Apr 4, 2025
@Evanraisul Evanraisul reopened this Apr 4, 2025
@dormando
Copy link
Collaborator
dormando commented Apr 4, 2025

I still see a bunch of unrelated changes and also looks like you may need to pull from upstream and rebase your branch? please carefully review what your commit looks like.

sorry it's a litle odd looking still

@Evanraisul
Update go mod
ae2365e 
Signed-off-by: Muhammad Raisul Islam Evan <raisul@appscode.com>
@Evanraisul
Add Memcached Authentication Support
7e4b290 
Signed-off-by: Muhammad Raisul Islam Evan <raisul@appscode.com>
@Evanraisul
Fix auth stuffs
1443d70 
Signed-off-by: Muhammad Raisul Islam Evan <raisul@appscode.com>
@Evanraisul
Copy link
Author

Sorry for the inconvenience. I think it is okay now.
Take a look.

dormando
dormando requested changes Apr 4, 2025
View reviewed changes
Copy link
Collaborator
@dormando dormando left a comment

Choose a reason for hiding this comment

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

getting closer, thanks!

@Evanraisul
Copy link
Author

Resolved those stuffs. I think it is now ready for merge.

@dormando
Copy link
Collaborator
dormando commented Apr 4, 2025

Thanks! It might take a while before we review this, so please have patience.

@dormando
Copy link
Collaborator
dormando commented Apr 4, 2025

Tests are missing which is the biggest obvious thing. Before I ask that you add tests I want to review it and see if this is the right approach though.

Thanks again!

@Evanraisul
Copy link
Author

Okay, thanks. Will wait for your review.

@dormando dormando added this to the Small protocol issues milestone Apr 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dormando dormando dormando requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Small protocol issues
Development

Successfully merging this pull request may close these issues.
2 participants
@Evanraisul @dormando
0