Skip to content
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

upgrade tubalmartin/cssmin to 4.1.0 #9928

Closed

Conversation

AntonEvers
Copy link
Contributor

@AntonEvers AntonEvers commented Jun 13, 2017

Description

According to https://github.com/tubalmartin/YUI-CSS-compressor-PHP-port/blob/master/README.md#v410-16-may-2017: Performance: 2x times faster than v4.0.0 after code profiling

Also minor enhancements like:

[target='_blank']       => [target=_blank]
[type="something"]      => [type=something]
background: transparent => background: 0 0
background: none        => background: 0 0
#f00                    => red
padding: 20px 20px      => padding 20px
0.12                    => .12

total min.css size reduced by 2563 bytes (from 1732225 to 1729662)

Fixed Issues (if relevant)

  1. none as if yet

Manual testing scenarios

  1. Before upgrading this package, generate minified css files, then run cat `find pub/static/ -name *.min.css -print | sort` > 3.0.0.min.css
  2. Upgrade the package, generate minified css again and run cat `find pub/static/ -name *.min.css -print | sort` > 4.1.0.min.css
  3. Compare the two files. you will see the enhancements like above.
  4. Visually compare the site before and after and see that there are no changes.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

Anton Evers added 2 commits June 13, 2017 13:35
According to https://github.com/tubalmartin/YUI-CSS-compressor-PHP-port/blob/master/README.md#v410-16-may-2017: Performance: 2x times faster than v4.0.0 after code profiling
Also minor enhancements like:

    [target='_blank']       => [target=_blank]
    [type="something"]      => [type=something]
    background: transparent => background: 0 0
    background: none        => background: 0 0
    #f00                    => red
    padding: 20px 20px      => padding 20px
    0.12                    => .12

total min.css size reduced by 2563 bytes (from 1732225 to 1729662)
@ishakhsuvarov ishakhsuvarov self-assigned this Jun 13, 2017
@ishakhsuvarov ishakhsuvarov added this to the June 2017 milestone Jun 13, 2017
@AntonEvers
Copy link
Contributor Author

@ishakhsuvarov how do you want to approach the failed integration test (INDEX=3). Is it good to first do the review and then change the minified css to compare against in

Magento\Framework\View\Asset\MinifierTest::testCSSminLibrary
Magento\Framework\View\Asset\MinifierTest::testCssMinification
Magento\Framework\View\Asset\MinifierTest::testDeploymentWithMinifierEnabled

or do you want me to change them right away in this PR?

@ishakhsuvarov
Copy link
Contributor

@ajpevers I will try to get someone from the Magento Frontend guys to review it.

magento-team pushed a commit that referenced this pull request Jun 15, 2017
 - Upgraded expected minification result for the test
@okorshenko
Copy link
Contributor

Hi @ajpevers your Pull Request has been merged to develop branch. Thank you

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.

3 participants