Conversation
mahhov
left a comment
mahhov
left a comment
There was a problem hiding this comment.
Thansk for catching the typos and improving the styles.
I made some comments on certain styling changes that I don't think are ideal..
| - `inline-environment-variables` replaces references to `process.env.<envVarName>` with their values in a JS file. | ||
|
|
||
| # Installation | ||
| --- |
There was a problem hiding this comment.
I'm not a fan of manually styling md's and would rather the md renderer do it's thing. Not only because I trust the renderer more than myself to decide when to add lines for best readability, but also because manually doing this could lead to consistency issues.
| ```js | ||
| // src/main.js | ||
| console.log('Welcome'); | ||
| console.log("Welcome"); |
There was a problem hiding this comment.
this isn't consistent with either the src nor js common style.
| ``` | ||
|
|
||
| `$ inline-scripts src/index.html out/index.html` | ||
| `$ inline-script-tags src/index.html out/index.html` |
| <!-- out/index.html --> | ||
| <p>Welcome</p> | ||
| <script>console.log('Welcome');</script> | ||
| <script> |
There was a problem hiding this comment.
Although this is easier to read, it's actually inaccurate. The script doesn't generate the new lines and indentation.
| <!-- src/index.html --> | ||
| <p>Welcome</p> | ||
| <link rel="stylesheet" href="style.css"> | ||
| <link rel="stylesheet" href="style.css" /> |
There was a problem hiding this comment.
I'm not a fan of unnecessary characters that don't improve readability.
| <!-- out/index.html --> | ||
| <p>Welcome</p> | ||
| <img src="data:image/png;base64, iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg=="> | ||
| <img |
There was a problem hiding this comment.
Like above, the script doesn't line-wrap, and I think the output snippet should reflect the actual behavior.
| add: (a, b) => a + b, | ||
| double: a => a * 2, | ||
| increment: a => a++, | ||
| square: (a) => a * a, |
There was a problem hiding this comment.
Like above, not a fan of the unnecessary ()
| All scripts usually take 2 parameters: the input and output files. | ||
|
|
||
| `$ inline-scripts src/index.html out/index.html .` | ||
| `$ inline-[script_type] src/index.html out/index.html` |
There was a problem hiding this comment.
for consistency, let's prefer dash-case to underscore.
2 participants
My first use on import following the instructions led to an error, and it took me a while to figure out. I later realised what I was doing wrong, and thought it would be to the advantage of others not to abandon this awesome module because of such errors. Apart from code edits, I made some aesthetic changes as well, while the others came out of good MD practices and prettier reformatting.