docs(devServer): clarify devServer "open" option description #1890
Conversation
Added information that you can also use browser name in the config file.
This removes a word that made a sentence not grammatically correct.
src/content/guides/production.md
Outdated
| + new webpack.DefinePlugin({ | ||
| + 'process.env.NODE_ENV': JSON.stringify('production') | ||
| + }) | ||
| ] | ||
| }) | ||
| }); |
There was a problem hiding this comment.
Hi @Simek, thanks for your contribution. As for webpack v4 we want users to rely on mode and not define manually the NODE_ENV variable since the default produciton mode does it for them.
Can you change this code snippet? After that I think it would be good to merge.
There was a problem hiding this comment.
Even if you can do some proof reading to the article and see if we can tip that feature, bonus points :)
There was a problem hiding this comment.
@Simek Any update on this? :)
There was a problem hiding this comment.
@jackfranklin @dhruvdutt Can you clarify what exact changes you would like me to implement in this PR? I've seen @chenxsan and @RasmusFonseca have added commits/small fixes there and your comments seems for me related to that portion of changes.
There was a problem hiding this comment.
My comment is that the guide suggests the user to manually configure uglify plugin and NODE_ENV while mode: 'production' does all that out of the box.
There was a problem hiding this comment.
@jeremenichelli I've know that since v4 there is a simplified mode option, but how's it's related to clarifying "open" option for devServer? This shouldn't be fixed or resolved in a separate Issue/PR?
There was a problem hiding this comment.
Ugh sorry GitHub is double highlighting the diff on the documentation so I though you were adding those lines, my bad. Also My handle @jeremenichelli, let's not spam Jack here 😉
|
@jeremenichelli It this one OK to merge? |
| @@ -325,7 +326,7 @@ __project__ | |||
| |- /node_modules | |||
| ``` | |||
|
|
|||
| With the loader configured and fonts in place, you can use incorporate them via an `@font-face` declaration. The local `url(...)` directive will be picked up by webpack just as it was with the image: | |||
| With the loader configured and fonts in place, you can incorporate them via an `@font-face` declaration. The local `url(...)` directive will be picked up by webpack just as it was with the image: | |||
There was a problem hiding this comment.
it should be ... a font face declaration, not an
|
@Simek Friendly ping |
|
@montogeek How could I help you? |
|
@Simek Regarding @jeremenichelli requested change. |
|
@montogeek Unfortunately I'm not the author of reviewed sentences or changes. If you are not able to fix that, I'm going to look on this later this evening. |
|
I see, it is out of the scope then. |
|
Thanks! |
Added information that you can also use browser name in the config file.