Skip to content

Use app version to generate scss filename - #8999

Merged
rullzer merged 1 commit into
masterfrom
css-js-name-based-on-apps-versions
Mar 28, 2018
Merged

Use app version to generate scss filename#8999
rullzer merged 1 commit into
masterfrom
css-js-name-based-on-apps-versions

Conversation

@skjnldsv

Copy link
Copy Markdown
Member

To force our scss cache to clear

juliusknorr
juliusknorr previously requested changes Mar 27, 2018
public function getCachedCSS(string $appName, string $fileName): ISimpleFile {
$folder = $this->appData->getFolder($appName);
return $folder->getFile($this->prependBaseurlPrefix($fileName));
$cachedFileName = $this->prependVersionPrefix($this->prependBaseurlPrefix($fileName), $appName);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like $folder->getFile() has been lost

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. Somehow missed that! 🙈

* @param string $appId
* @return string
*/
private function prependVersionPrefix(string $cssFile, string $appId): string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason to not just have one prependPrefix method?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplicity? :p

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, don't mind that much 😉

@skjnldsv
skjnldsvforce-pushed the css-js-name-based-on-apps-versions branch from feb1784 to 7ebb686CompareMarch 27, 2018 20:03
@codecov

codecovBot commented Mar 27, 2018

Copy link
Copy Markdown

Codecov Report

Merging #8999 into master will decrease coverage by 16.92%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #8999 +/- ##
=============================================
- Coverage 51.96% 35.03% -16.93% - Complexity 25278 25283 +5 
=============================================
Files 1601 1601 Lines 94950 94965 +15 Branches 1388 1388 =============================================
- Hits 49342 33273 -16069 - Misses 45608 61692 +16084
Impacted FilesCoverage ΔComplexity Δ
lib/private/Template/SCSSCacher.php6.2% <0%> (-65.54%)38 <2> (+2)
...te/Authentication/Token/DefaultTokenCleanupJob.php0% <0%> (-100%)1% <0%> (ø)
...ty/Exceptions/CrossSiteRequestForgeryException.php0% <0%> (-100%)1% <0%> (ø)
lib/private/App/AppStore/Version/Version.php0% <0%> (-100%)3% <0%> (ø)
lib/public/RichObjectStrings/Definitions.php0% <0%> (-100%)2% <0%> (ø)
apps/dav/lib/CalDAV/Activity/Setting/Todo.php0% <0%> (-100%)8% <0%> (ø)
apps/dav/lib/CalDAV/Activity/Setting/Event.php0% <0%> (-100%)8% <0%> (ø)
lib/private/Security/IdentityProof/Key.php0% <0%> (-100%)3% <0%> (ø)
lib/private/Files/SimpleFS/SimpleFile.php0% <0%> (-100%)13% <0%> (ø)
lib/private/Remote/InstanceFactory.php0% <0%> (-100%)2% <0%> (ø)
... and 530 more

Fixed scsscacher tests
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
skjnldsvforce-pushed the css-js-name-based-on-apps-versions branch from 7ebb686 to 1d7b14dCompareMarch 28, 2018 06:34
@skjnldsv

Copy link
Copy Markdown
MemberAuthor

Fixed tests

@juliusknorrjuliusknorr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works as expected.

* @param string $appId
* @return string
*/
private function prependVersionPrefix(string $cssFile, string $appId): string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, don't mind that much 😉

@rullzer
rullzer merged commit cdb2cc1 into masterMar 28, 2018
@rullzer
rullzer deleted the css-js-name-based-on-apps-versions branch March 28, 2018 13:59
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsfeature: scss

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@skjnldsv@rullzer@juliusknorr