Skip to content

style(utils/addEntries): cleaner variable naming #1478

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

Merged
merged 3 commits into from
Aug 29, 2018

Conversation

michael-ciniawsky
Copy link
Member

Type

  • Style

Issues

  • None

SemVer

  • Patch

const domain = createDomain(devServerOptions, app);
const devClient = [`${require.resolve('../../client/')}?${domain}`];
const domain = createDomain(options, app);
const entries = [ `${require.resolve('../../client/')}?${domain}` ];
Copy link
Member

Choose a reason for hiding this comment

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

Looks your have here, maybe better run prettier?

Copy link
Member Author

Choose a reason for hiding this comment

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

The space is intenentional [ a, b ] here, prettier does hopefully leave it :)

Copy link
Member

Choose a reason for hiding this comment

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

@michael-ciniawsky hm, we should integrate prettier here 😕

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, prettier will be included with webpack-defaults in webpack-dev-server >= v4.0.0, it should still leave whitespace in this case since it is more readable

Copy link
Member

Choose a reason for hiding this comment

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

@alexander-akait
Copy link
Member

@michael-ciniawsky looks appveyor fail maybe need rebase?

@codecov
Copy link

codecov bot commented Aug 29, 2018

Codecov Report

Merging #1478 into master will increase coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1478      +/-   ##
==========================================
+ Coverage   74.02%   74.17%   +0.15%     
==========================================
  Files          10       10              
  Lines         666      666              
==========================================
+ Hits          493      494       +1     
+ Misses        173      172       -1
Impacted Files Coverage Δ
lib/utils/addEntries.js 100% <100%> (ø) ⬆️
lib/Server.js 81.71% <0%> (+0.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ab9eb6...5a74933. Read the comment docs.

@michael-ciniawsky
Copy link
Member Author

Yep :)

@michael-ciniawsky michael-ciniawsky merged commit 2d35287 into master Aug 29, 2018
@michael-ciniawsky michael-ciniawsky deleted the refactor/utils/addEntries branch August 29, 2018 12:00
@michael-ciniawsky michael-ciniawsky removed this from the 3.1.7 milestone Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants