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

Keepalive #17

Merged
merged 2 commits into from
Apr 6, 2015
Merged

Keepalive #17

merged 2 commits into from
Apr 6, 2015

Conversation

ryscheng
Copy link
Member

@ryscheng ryscheng commented Apr 3, 2015

Fixes #15

Note: browserify 9.0.5 seems to be broken, so I'm forcing a ceiling in our dependencies

@ryscheng
Copy link
Member Author

ryscheng commented Apr 3, 2015

@soycode PTAL

@ryscheng ryscheng mentioned this pull request Apr 3, 2015
@@ -16,25 +16,21 @@
* @param {WebSocket} webSocket Alternative webSocket implementation for tests
**/
var DEBUGLOGGING = false;
var WS_URL = 'ws://localhost:8082/route/';
//var WS_URL = 'wss://data.radiatus.io/route/';
Copy link

Choose a reason for hiding this comment

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

It looks like hardcoding it here changes the logic a bit, i.e. makes it so WS_URL is always localhost rather than being localhost when DEBUG is true and radiatus.io otherwise. If that's intentional I'm okay with that, but it may still be nice to have some sort of flag to select or pass in a custom WS host.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. The code we had before was defacto hardcoded, because the DEBUG variable is never set or passed in anywhere. So this was just to clear up the code

We do want some way to configure the endpoint (e.g.dist vs testing) and my issue here will track that
#16

@agallant
Copy link

agallant commented Apr 3, 2015

👍

ryscheng added a commit that referenced this pull request Apr 6, 2015
@ryscheng ryscheng merged commit dabb42b into master Apr 6, 2015
@ryscheng ryscheng deleted the ryscheng-keepalive branch April 6, 2015 19:45
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.

Keepalive Ping
2 participants