Skip to content

fix(server): Change clientLogLevel order to be called first#1973

Merged
hiroppy merged 2 commits into
masterfrom
feature/fix-client-log
Jun 5, 2019
Merged

fix(server): Change clientLogLevel order to be called first#1973
hiroppy merged 2 commits into
masterfrom
feature/fix-client-log

Conversation

@hiroppy

@hiroppy hiroppy commented Jun 5, 2019

Copy link
Copy Markdown
Contributor
  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

yes

Motivation / Use-Case

maybe related #1972.

before

Screen Shot 2019-06-05 at 16 18 45

after

Screen Shot 2019-06-05 at 16 21 26

Breaking Changes

no

Additional Info

@hiroppy hiroppy requested a review from alexander-akait as a code owner June 5, 2019 14:38

@alexander-akait alexander-akait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All good, one note

Comment thread lib/Server.js Outdated
}

if (this.options.liveReload !== false) {
if (this.options.liveReload) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can be breaking change for non boolean value, let's avoid this in this PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, I'll change it at next major version.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes

@hiroppy hiroppy force-pushed the feature/fix-client-log branch from 239150a to fcf85c2 Compare June 5, 2019 14:48
@hiroppy

hiroppy commented Jun 5, 2019

Copy link
Copy Markdown
Contributor Author

Sorry, I contained another commit to put .gitkeep.

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