uperl / AnyEvent-WebSocket-Client

WebSocket client for AnyEvent

9 stars 10 forks source link

Unix Domain Socket Support #42

Closed akalinux closed 5 years ago

akalinux commented 5 years ago

As per our email chain.

I have done the following.

Added a new constructor argument, unix_socket
Updated AnyEvent;:WebSocket::Client::connect
Created unix domain socket unit tests

I also found pod errors/warnings and cleaned those up as well.

plicease commented 5 years ago

The test should be named anyevent_websocket_client__unix.t or similar since it is a test of the connection class.

akalinux commented 5 years ago

I think that is the last of the changes. Anything else?

akalinux commented 5 years ago

I would like to say thank you for being so responsive, not many authors are.;)On Jan 14, 2019 5:33 AM, Graham✈️✈️ notifications@github.com wrote:@plicease commented on this pull request.

In lib/AnyEvent/WebSocket/Client.pm:

});

uncomment to enter the event loop before exiting.

Note that calling recv on a condition variable before

it has been triggered does not work on all event loops

AnyEvent->condvar->recv;

+# fixed bad bad pod encoding + +=encoding utf8

pod testing happens after dzil builds the dist: xt/author/pod.t .................. ok
xt/author/pod_coverage.t ......... ok
xt/author/pod_spelling_common.t .. ok
xt/author/pod_spelling_system.t .. ok

It's sort of pointless to test the incomplete POD.

—You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or mute the thread.

akalinux commented 5 years ago

Following up?

plicease commented 5 years ago

The more I think about it, the more I would like to implement this like so:

https://github.com/plicease/AnyEvent-WebSocket-Client/pull/45

Reasons:

The interface is immediately familiar to someone who having used AnyEvent::Socket. The API has its drawbacks, but it is sufficient and doesn't require learning something new for those who already know it.
The maintenance is much simpler. Because we don't own the interface for specifying the host/port combination it is no longer our responsibility to make sure that it works with a unix domain socket, etc., that is correctly the responsibility of AnyEvent::Socket. The tests for my proposal are as a result much simpler since we only need to make sure that the arguments passed into connect are used with tcp_connect.

akalinux commented 5 years ago

Well I currently have both implemented. So?

akalinux commented 5 years ago

Meh,

Trimmed everything down: --- removed constructor option -- removed unix=>socket option

Simplified the unit tests a bit