Add app version to JS and CSS #11591
Conversation
LukasReschke
force-pushed
the
addAppVersionToCSSandJS
branch
from
32e7c11
to
ddfcf53
Compare
|
looks good 👍 |
| if(empty(self::$versionHash)) { | ||
| $appVersions = array(); | ||
| foreach(OC_App::getAllApps() as $app) { | ||
| $appVersions[] = OC_App::getAppVersion($app); |
There was a problem hiding this comment.
I hope this loop doesn't have any performance impact.
Since the $versionHash is stored statically it will compute it only once per request, but will likely recompute it for every requested resource.
Would it make sense to store it in the session ?
There was a problem hiding this comment.
That makes sense. - Yes.
There was a problem hiding this comment.
Mhm, actually I don't know how we should get informed about new app versions then... - We therefore should better invest time to cache this action inside OC_App... - Let's make this in another PR.
There was a problem hiding this comment.
Agreed.
CC @icewind1991 for a second opinion
There was a problem hiding this comment.
\OC_App::getAppVersions()No need to do a loop
There was a problem hiding this comment.
Fixed. Thanks @icewind1991
|
Tested, works. See comment. |
LukasReschke
force-pushed
the
addAppVersionToCSSandJS
branch
from
b54eafc
to
e3ff57c
Compare
|
👍 |
This leads to the regeneration of the hash in case a single application is updated. Fixes #11374
LukasReschke
force-pushed
the
addAppVersionToCSSandJS
branch
from
e3ff57c
to
6ccda2a
Compare
|
|
|
Mhm - something is messed up with him completely... - It's not even in this PR. |
|
@owncloud-bot Retest this please. |
|
After this is merged and backported also #11556 should be backported. |
|
agreed |
I'll add the required logic regarding the asset pipeline. |
|
🚀 Test PASSed. 🚀 |
@LukasReschke done |
|
👍 - let's wait for Jenkins |
|
I wonder if it would make sense to add this repair step to this branch as well: https://github.com/owncloud/core/pull/11430/files#diff-d3585657ca758697141bd56c5aa79a9aR14 It clears the assets folder on upgrade ... |
|
The inspection completed: 2 new issues |
|
🚀 Test PASSed. 🚀 |
Add app version to JS and CSS
|
Stable7: 6e3a7ea...af72528 |
|
What about backporting the htaccess PR? |
|
Feel free to do it - I will not touch it, for me this seems too risky ;-) |
|
Okay. Then not:) |
This leads to the regeneration of the hash in case a single application is updated. - This does not touch the Assetic Pipeline because I don't know how it works.
Fixes #11374
@karlitschek Shall we backport this?