Skip to content
This repository was archived by the owner on Jul 13, 2021. It is now read-only.

Conversation

@JSteunou
Copy link

@JSteunou JSteunou commented Jul 2, 2018

  • add options {server: {host, port}, client: {host, port, secure}}
  • secondary effect options.server can be a server instance or a definition object
  • all options are set in importance order, compatiblity is preserved (at least it should be)
  • for clarity introduce websocketServer & websocketClient computed options

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

see #79

Breaking Changes

Trying my best not to.

Additional Info

* add options `{server: {host, port}, client: {host, port, secure}}`
* secondary effect options.server can be a server instance or a definition object
* all options are set in importance order, compatiblity is preserved (at least it should be)
* for clarity introduce websocketServer & websocketClient computed options
@shellscape
Copy link
Contributor

Thanks for the PR 🍺

While that's not exactly the implementation we're going for (and the changes in the client scripts would be considered breaking), I'm only going to put a hold tag on this. #80 is more along the lines of what we'd be going towards for allowing host and port for the WebSocket Server.

We haven't quite arrived at how we're going to work with the actual server yet though. So again I don't want to close this, but we can't accept it right this moment and you might have to do some rebasing soon.

@JSteunou
Copy link
Author

JSteunou commented Jul 2, 2018

Oopsy I was in a rush and did not see #80

No problem, I was testing things because we just need this feature right now, so why not sharing you know...

@shellscape
Copy link
Contributor

Absolutely! Glad you did!

@michael-ciniawsky michael-ciniawsky changed the title [WIP] Add support for a split server / client configuration [WIP] feat: add support for a split server / client configuration Sep 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants