-
Notifications
You must be signed in to change notification settings - Fork 5
Less.js 4.2.0 and magento 2.4.6 without any less modifications (discussion) #9
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
Comments
Hey @piotrkwiecinski: I'm not very keen on introducing configuration settings in this module. As it would need to work as well without a database connection. So it then it should be a config setting in the An alternative approach to the current patch you are using, is that you use the changes to the less files as a patch from this PR: https://github.com/magento/magento2/pull/38335/files, those changes should also fix your problem with less.js v4 I believe, and already got included in Magento 2.4.7. |
@hostep thank you for the fast reply. We'll have a look. I agree with keeping this module lean. Ultimately it's an issue with magento core. I was thinking more about something tiny like:
|
And thank you for an awesome module. Our initial tests shows it shaved ~3.5 min from a deployment run on Github action. |
Hi @piotrkwiecinski, could you give the changes from #10 a try? You can use the Are you okay with this solution? It's not using environment variables though. |
Hi @hostep. We already push config.php to repo to build theme in CI so it seems like a good solution. |
Hey @piotrkwiecinski, this feature is now released in version 1.6.0 Thanks for the suggestions! |
Hi @hostep,
we are playing with this extensions and found a way to get less to compile without any changes to core magento less.
Setting math to always restores a behaviour from 3.X based on https://lesscss.org/usage/#less-options-math
Here is a dirty hack we used.
Do you have an idea how we could make it more configurable? Maybe we could pass a parameter somehow via environment variables or deployment config.
The text was updated successfully, but these errors were encountered: