OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
863 stars 438 forks source link

Added filemtime to merged JS/CSS hash calculation algorithm #4004

Closed boesbo closed 1 month ago

boesbo commented 1 month ago

Description (*)

Add the timestamp of the files in the MD5 HASH generation criteria of the files.

Fixed Issues (if relevant)

  1. Fixes OpenMage/magento-lts#4003

Questions or comments

Contribution checklist (*)

boesbo commented 1 month ago

If you can I ask you for a review @fballiano

boesbo commented 1 month ago

@fballiano I improved the PR. There was some confusion with the variable name, so your question was legitimate. I have made the code clearer and accepted your suggestion not to run filemtime() several times;

fballiano commented 1 month ago

@boesbo great, I'll test it out a bit

boesbo commented 1 month ago

Non mi piace il modo in cui funziona l'unione per impostazione predefinita in OM, ma questo è un altro argomento, ho testato questo PR e fa quello che afferma e penso che sia ok unire

It is only an improvement of the current functionality in view of a modern (now current) management of these assets in the browser cache. Thx!

boesbo commented 1 month ago

If you can I ask you for a review @kasper-agg

fballiano commented 1 month ago

I'll merge this in a couple of days if there's none against

kasper-agg commented 1 month ago

If you can I ask you for a review @kasper-agg

What @fballiano already mentioned, the whole merging thing is another topic, your PR fixes the current issue at hand :+1: