-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/wss #5
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
Feature/wss #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
No functional problems. Have some questions, but they don't need to be resolved for this to be merged (i.e. can always be resolved later, if needed).
) { | ||
// Store the new connection in $this->clients | ||
$this->clients[$connection->getRemoteName()] = $connection; | ||
echo "> [{$connection->getRemoteName()}] Client connected {$request->getUri()}\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of readability, it might be worth using printf
instead of echo
. Consider:
vprintf("> [%s] Client connected %s\n", [
$connection->getRemoteName(),
$request->getUri(),
]);
); | ||
|
||
$server->run(); | ||
$server = new SolidPubSub($options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some later point we might want to split the config, class, and call, but until we start adding unit-tests, I don't think it is worth the effort (yet).
No description provided.