-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
refactor: server #1847
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
refactor: server #1847
Conversation
} else { | ||
} | ||
|
||
if (!certExists) { |
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.
Regression in previously refactor, need add tests (will be done in other PR)
); | ||
} | ||
|
||
setupFeatures() { |
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.
No options, we in class so we can use this
|
||
(this.options.features || defaultFeatures).forEach((feature) => { | ||
features[feature](); | ||
}); |
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.
Also we initialize features in this method (all in one place)
Codecov Report
@@ Coverage Diff @@
## master #1847 +/- ##
==========================================
- Coverage 89.25% 89.22% -0.03%
==========================================
Files 10 10
Lines 614 622 +8
Branches 186 187 +1
==========================================
+ Hits 548 555 +7
- Misses 54 55 +1
Partials 12 12
Continue to review full report at Codecov.
|
/cc @hiroppy I merge to continue working and do release, feel free to feedback and open a issue/send a PR if you find something wrong |
For Bugs and Features; did you add new tests?
Not required
Motivation / Use-Case
Refactor code (part 1 of 3)
Breaking Changes
No
Additional Info
No