Skip to content

Move completeUpgrade to WebSocketServer.prototype#897

Merged
3rd-Eden merged 1 commit intomasterfrom
move/complete-upgrade
Nov 17, 2016
Merged

Move completeUpgrade to WebSocketServer.prototype#897
3rd-Eden merged 1 commit intomasterfrom
move/complete-upgrade

Conversation

@lpinca
Copy link
Member

@lpinca lpinca commented Nov 17, 2016

This brings the following changes:

  • Merge the upgrade function into WebSocketServer.prototype.handleUpgrade().
  • Extract completeUpgrade from upgrade and add it to WebSocketServer.prototype.

@lpinca lpinca force-pushed the move/complete-upgrade branch from 69e62df to 703986f Compare November 17, 2016 14:49
@lpinca lpinca force-pushed the move/complete-upgrade branch from 703986f to 40291cb Compare November 17, 2016 16:44
@3rd-Eden
Copy link
Member

Things are breaking :9

@lpinca
Copy link
Member Author

lpinca commented Nov 17, 2016

It's coveralls which is drunk, tests pass :)

@3rd-Eden
Copy link
Member

@lpinca Yeah, I guess it complains when coverage gets reduced somehow.

@3rd-Eden 3rd-Eden merged commit 64eba5e into master Nov 17, 2016
@3rd-Eden 3rd-Eden deleted the move/complete-upgrade branch November 17, 2016 21:22
@lpinca
Copy link
Member Author

lpinca commented Nov 17, 2016

master branch (before merge)

=============================== Coverage summary ===============================
Statements   : 91.52% ( 1111/1214 )
Branches     : 88.75% ( 686/773 )
Functions    : 95.92% ( 94/98 )
Lines        : 92.4% ( 1045/1131 )
================================================================================

this branch

================================ Coverage summary ===============================
Statements   : 91.49% ( 1108/1211 )
Branches     : 88.72% ( 684/771 )
Functions    : 95.92% ( 94/98 )
Lines        : 92.37% ( 1041/1127 )
================================================================================

kinda weird as coverage is exactly the same.

@3rd-Eden
Copy link
Member

Well, the statements, branches and lines have less coverage in this branch.

@3rd-Eden
Copy link
Member

But what ever, it's not that important.

@lpinca
Copy link
Member Author

lpinca commented Nov 17, 2016

If you compare it with the total statements it's the same.
I mean, there are 3 less statements now so those 3 statements previously covered are no longer covered.
Percentage is indeed lower.

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.

2 participants