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

Fix the socket.io breaking client applications due to default path used #136

Merged
merged 6 commits into from
Feb 23, 2018
Merged

Conversation

noxxious
Copy link
Contributor

@noxxious noxxious commented Jan 6, 2018

This fixes the issue #109.

@codecov-io
Copy link

codecov-io commented Jan 6, 2018

Codecov Report

Merging #136 into master will decrease coverage by 5.55%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
- Coverage   63.88%   58.33%   -5.56%     
==========================================
  Files           2        2              
  Lines         288      288              
==========================================
- Hits          184      168      -16     
- Misses        104      120      +16
Impacted Files Coverage Δ
lib/appmetrics-dash.js 58.18% <100%> (-5.58%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e524d8f...1fdd67a. Read the comment docs.

@robbinspg
Copy link
Contributor

robbinspg commented Feb 15, 2018

@noxxious Please can you read the contribution guidelines and if in agreement add your name to the AUTHORS.md to acknowledge acceptance so that we can accept this PR

Copy link
Contributor

@robbinspg robbinspg left a comment

Choose a reason for hiding this comment

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

Can you alter the comment lines at lib/appmetrics-dash.js 133/134 to

 // Specify a path, to not collide with user's socket.io.

@noxxious
Copy link
Contributor Author

Updated according to the comments.

@robbinspg robbinspg merged commit 6689a7d into RuntimeTools:master Feb 23, 2018
robbinspg added a commit that referenced this pull request Apr 26, 2018
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