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

Add support for UNIX domain sockets #120

Merged
merged 32 commits into from
Dec 11, 2023
Merged

Conversation

kentslaney
Copy link
Contributor

interface for memcached -s

src/Connection.cpp Outdated Show resolved Hide resolved
@kentslaney kentslaney changed the title Added support for UNIX domain sockets Add support for UNIX domain sockets Nov 19, 2023
@kentslaney kentslaney changed the title Add support for UNIX domain sockets [WIP] Add support for UNIX domain sockets Nov 19, 2023
@kentslaney kentslaney changed the title [WIP] Add support for UNIX domain sockets Add support for UNIX domain sockets Nov 28, 2023
@kentslaney kentslaney marked this pull request as draft November 28, 2023 20:47
@kentslaney kentslaney marked this pull request as ready for review November 28, 2023 21:52
@kentslaney
Copy link
Contributor Author

kentslaney commented Nov 29, 2023

I think this is ready to be squash and merged. I can rebase the branch and force push myself if that's easier.

The only existing interface I modified was connection::connectPoll which is protected anyway.

The other notable change is that if you're using make test instead of the /misc/travis scripts, test_unix.cpp expects that memcached was started using ./misc/memcached startall instead of ./misc/memcached start.

The PR workflow is still awaiting approval, but in the meantime the forked workflow results can be compared to the origin/HEAD workflow baseline. I still haven't gotten act running properly which is why there were so many commits towards the end.

@lexdene
Copy link
Contributor

lexdene commented Nov 29, 2023

I have fixed ci on master branch.

Would you please merge (or rebase) master first to see if unittest can pass?

@pypeng
Copy link
Contributor

pypeng commented Nov 30, 2023

need a version bump, like this: #116

@kentslaney kentslaney marked this pull request as draft November 30, 2023 21:23
@kentslaney kentslaney marked this pull request as ready for review December 1, 2023 00:35
@kentslaney kentslaney marked this pull request as draft December 1, 2023 01:03
@kentslaney kentslaney marked this pull request as ready for review December 1, 2023 04:41
@kentslaney
Copy link
Contributor Author

kentslaney commented Dec 1, 2023

The server string implementation got more complicated than I expected and there's still dead code in there. It should get optimized out by the compiler. I don't think it'll present notable maintenance overhead, but would also understand a refactor request.

@kentslaney
Copy link
Contributor Author

After looking more closely at the default branch I realized there was a reason it was parsed the way it was. The PR implementation is still more complicated than the initial binding language solution but seemed worth it for the guaranteed language consistency. There is no longer dead code.

@kentslaney kentslaney requested a review from lexdene December 9, 2023 06:04
@kentslaney
Copy link
Contributor Author

kentslaney commented Dec 11, 2023

One more bump on workflow approval please @lexdene (I assume? Can't tell who approved it). Looks like this was the issue. Works on my fork this time, should have caught the last one.

@lexdene lexdene merged commit d32820f into douban:master Dec 11, 2023
28 checks passed
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.

3 participants