Skip to content

update "tubalmartin/cssmin" to make CSS minifying compatible with calc() #9447

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

Closed
frenky2334 opened this issue Apr 28, 2017 · 13 comments
Closed
Labels
Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed

Comments

@frenky2334
Copy link

Hi!
I tried update composer to update "tubalmartin/cssmin" module. But in update progress i get the next one message which you can see at the attached screenshot.
cssminify
The composer looks like this:
composer
Please, help me!

@hostep
Copy link
Contributor

hostep commented Apr 28, 2017

Remove tubalmartin/cssmin from your composer.json file, Magento already includes an older version, so it will cause conflicts.

Also: next time, use the community forum or https://magento.stackexchange.com/ or something like that, because this isn't really a bug :)

@frenky2334
Copy link
Author

But i need newer version of this module because in new versions the bug with the calc() property is fixed

@sambolek
Copy link
Contributor

Hi @frenky2334
this pull request updates cssmin and fixes css calc() issues #9027 (comment)

@korostii
Copy link
Contributor

korostii commented May 3, 2017

this isn't really a bug :)

I'm sorry, how exactly an outdated dependency "isn't really a bug"?
Is that idea written somewhere in the guidelines? Should all those "update ZF dependencies" issues get closed? Maybe let's also drop the issue about updating the composer as well while we're at it?

It might be "not a priority", but it's still a valid technical issue.

@sambolek
Copy link
Contributor

sambolek commented May 3, 2017

Current M2 releases are made to work with previous versions.

As CSSmin maintainer changed the way the library is called, you cannot simply update the library version and hope everything will work. That's why version in composer.json is set to a specific one.

The library was updated in the pull request I linked above. You might be able to create a patch and apply it or wait for a new M2 release where it'll probably be merged.

@frenky2334
Copy link
Author

Hi @sambolek

Thanks for the answer.
I look at pull request which you recommended me. I have one question. Where can I get the updated library files?

@sambolek
Copy link
Contributor

sambolek commented May 3, 2017

Incorporate all changes from pull request (including composer.json) and run "composer install" or "composer update tubalmartin/cssmin".

@frenky2334
Copy link
Author

@sambolek
In your pull request you use 2.2.0-dev version, but in our production is 2.1.6
The difference in versions will not lead to the breakdown of the site?
Or better not risk for update this module and wait the official update magento?

@sambolek
Copy link
Contributor

sambolek commented May 4, 2017

If you want to be 100% sure, wait for next release :)

@frenky2334
Copy link
Author

@sambolek
Thanks )

@korostii
Copy link
Contributor

korostii commented May 4, 2017

If you want to be 100% sure, wait for next release :)

Just a heads up: there is no guarantee it will appear in the next 2.1.x release (or any of the 2.1.x releases at all, for that matter), and timelines for the future releases aren't provided by Magento, Inc.
So, potentially, you might need to wait for months until it gets released in 2.2.
Also, making an extra pull request targeting 2.1-develop should somewhat speed up that process (as explained here by a Magento employee).

@magento-engcom-team magento-engcom-team added the Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed label Sep 11, 2017
@magento-engcom-team
Copy link
Contributor

@frenky2334 Closing this issue as it is not a Magento bug, tubalmartin/cssmin is already bundled with magento version 4.1.0.

Thank you.

@korostii
Copy link
Contributor

korostii commented Apr 24, 2018

Closing this issue as it is not a Magento bug

I beg your pardon? By using 3rd-party code inside Magento, you effectively start owning the 3rd-party issues as well. It's just the same with bundling the fotorama.js into Magento: you add some bugs by doing that, and fixing them (if, say, that 3rd-party code isn't maintained any more) might require to either your own fork of said code (as it was done with zend framework 1) or replacing such a library altogether.

But it sure is a Magento bug, albeit it is already fixed by a @sambolek on the 2.2.x branch (many thanks to him for that). IMHO we're also lucky this is one of those issues where updating the bundled library (and fixing a few lines of code invoking it as well as couple tests - in this particular case) is enough to fix a bug.

tubalmartin/cssmin is already bundled with magento version 4.1.0

For whomever seeing this issue in their installations: yes, tubalmartin/cssmin of version 4.x is currently bundled, but only on the 2.2.x releases.

UPD.: 2.1.x branch received the fix in version 2.1.15, "tubalmartin/cssmin" got updated to "3.0.0".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed
Projects
None yet
Development

No branches or pull requests

5 participants