-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(server): add transportMode #2116
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
clientMode
andserverMode
testsMotivation / Use-Case
#1860
This replaces
clientMode
andserverMode
options with the usage:Breaking Changes
Experimental options
clientMode
andserverMode
removedAdditional Info
transportMode-option.test.js
is bulky and maybe should be broken downServer.js
could definitely be moved into some unifiedupdateOptions
helper to be tested more easily.