Output CSS in /static/css and switch to using stylus directly#2544
Output CSS in /static/css and switch to using stylus directly#2544phillipj merged 2 commits intonodejs:masterfrom XhmikosR:master-xmr-css-target
/static/css and switch to using stylus directly#2544Conversation
Aissaoui-Ahmed
requested review from
Fishrock123,
SEWeiTung and
diagramatics
|
@nodejs/website This could use reviews. |
| const labelForBuild = '[stylus] static/css finished' | ||
| console.time(labelForBuild) | ||
|
|
||
| fs.mkdir(path.join(__dirname, 'build'), () => { |
There was a problem hiding this comment.
Did you consider the { recursive: true } option of fs.mkdir() to avoid the nested callbacks below? As described by docs;
// Creates /tmp/a/apple, regardless of whether `/tmp` and /tmp/a exist.
fs.mkdir('/tmp/a/apple', { recursive: true }, (err) => {
if (err) throw err;
});
There was a problem hiding this comment.
Yeah, but I thought this change should be made separately after all the other changes have settled :)
There was a problem hiding this comment.
Also, this requires Node.js 10, another reason I didn't make this change yet.
There was a problem hiding this comment.
Is that requirement a problem though?
As far as I see it, we're running Node.js 10 in CI and $latest when building on the hosting server nodejs/build/setup/www/resources/scripts/build-site.sh.
There was a problem hiding this comment.
No problem for me, it's just that such change should be a clear patch IMO and not sneaked in in other patches, that is all.
|
Really cool to get rid of seemingly unnecessary abstraction to use stylus 💯 |
4 participants
Draft because it requires #2461 and #2462.Fixes #2467