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

Client stats #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Client stats #124

wants to merge 1 commit into from

Conversation

dragonsinth
Copy link

Hi @bradfitz I noticed that the author of #63 closed their PR. I'm really hoping to get this landed, so I reworked it a bit, addressed your original comments, and added some test coverage.

@@ -30,7 +30,7 @@ import (
"time"
)

const testServer = "localhost:11211"
const testServer = "127.0.0.1:11211"
Copy link
Author

Choose a reason for hiding this comment

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

NB: I had to use this to connect to my loopback-only memcached (we don't typically want local services to bind to external interfaces). I can revert this line if you want, but thought it might actually be nice to include.

@dragonsinth dragonsinth mentioned this pull request Sep 16, 2020
@dragonsinth
Copy link
Author

ping? I think I addressed all your feedback in the original PR :)

@jlisthood
Copy link

This feature is useful, thanks for taking the time to add it. I hope it gets merged some day

@bradfitz bradfitz removed the cla: yes label Sep 4, 2023
@franklinlindemberg
Copy link

Hi @bradfitz , do you have any update if there's anything blocking this PR?

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.

None yet

5 participants