-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(server): add transportMode #2116
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
Conversation
|
|
||
| this.log.warn( | ||
| 'clientMode is an experimental option, meaning its usage could potentially change without warning' | ||
| 'transportMode is an experimental option, meaning its usage could potentially change without warning' |
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.
Maybe we can move this in util, like getTransportMode(options: object): object
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.
Looks good, need fix CI problems
Codecov Report
@@ Coverage Diff @@
## master #2116 +/- ##
==========================================
+ Coverage 94.56% 94.59% +0.02%
==========================================
Files 32 32
Lines 1215 1221 +6
Branches 340 344 +4
==========================================
+ Hits 1149 1155 +6
Misses 64 64
Partials 2 2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #2116 +/- ##
==========================================
+ Coverage 96.07% 96.08% +<.01%
==========================================
Files 34 34
Lines 1224 1227 +3
Branches 346 349 +3
==========================================
+ Hits 1176 1179 +3
Misses 47 47
Partials 1 1
Continue to review full report at Codecov.
|
|
@evilebottnawi @hiroppy What is the cause of the Lint |
|
@Loonride i think we need update |
|
I created a pr which updated package-lock.json. #2123 |
0858045 to
84daa27
Compare
84daa27 to
c648678
Compare
|
/cc @Loonride something wrong with CI 😕 Looks need update snapshots or not |
|
@evilebottnawi Looks like unlucky e2e test run. Maybe there is a way to let |
|
/cc @Loonride what is status, i think we should merge this PR and do release |
|
@evilebottnawi Ready for merge. |
|
@Loonride we need something any for CI fix? |
For Bugs and Features; did you add new tests?
Yes, based on moving around and changing existing
clientModeandserverModetestsMotivation / Use-Case
#1860
This replaces
clientModeandserverModeoptions with the usage:Breaking Changes
Experimental options
clientModeandserverModeremovedAdditional Info
transportMode-option.test.jsis bulky and maybe should be broken downServer.jscould definitely be moved into some unifiedupdateOptionshelper to be tested more easily.