-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Remove comments from the *built* CSS files #17035
Remove comments from the *built* CSS files #17035
Conversation
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/06b61e04ab1e9b2/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/06b61e04ab1e9b2/output.txt Total script time: 1.50 mins Published |
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/7ec583dc73f1a55/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/61e9a9b45861b15/output.txt |
What is the improvement in terms of size reduction? |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/61e9a9b45861b15/output.txt Total script time: 5.44 mins
|
Comparing
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/7ec583dc73f1a55/output.txt Total script time: 17.91 mins
|
bf15b92
to
7702479
Compare
The old pre-processor used for CSS, and HTML, files leaves comments intact which unnecessarily contributes to the overall size of the *built* CSS files (note that the built JavaScript files don't include comments). Rather than trying to "hack" comment removal into the pre-processor it seems easier to use a PostCSS plugin instead. The one potential issue is that it also affects *some* whitespaces, and it's not clear to me if this'll work with the various CSS-related tests that run in mozilla-central. Please refer to https://www.npmjs.com/package/postcss-discard-comments for additional information.
7702479
to
7413546
Compare
Could you make a patch for m-c (no need to file a bug for that: |
Sure, please see https://phabricator.services.mozilla.com/D189455 |
CI is green (especially stylelint): |
Nice, thank you! So, does that mean that this PR is good to go? |
Yep I just forgot to do it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
Just FYI, the |
The old pre-processor used for CSS, and HTML, files leaves comments intact which unnecessarily contributes to the overall size of the built CSS files (note that the built JavaScript files don't include comments).
Rather than trying to "hack" comment removal into the pre-processor it seems easier to use a PostCSS plugin instead. The one potential issue is that it also affects some whitespaces, and it's not clear to me if this'll work with the various CSS-related tests that run in mozilla-central.
Please refer to https://www.npmjs.com/package/postcss-discard-comments for additional information.