This issue has novice tasks. Therefore if you are an experienced core developer and have multiple commit mentions it is appreciated if instead of doing the task, you review the task done by a novice. Experienced people giving feedback is very valuable.

Problem/Motivation

Followup from #1824778: Convert css_gzip_compression and js_gzip_compression variables to CMI system

Proposed resolution

  • Swap extension_load() and the config() calls? - this would save loading the configuration object if it's never going to needed anyway. This needs to be done in drupal_page_set_cache() and \Drupal\Core\Asset\AssetDumper::dump().
  • Remove section from settings.php about CSS/JS aggregated file gzip compression as we now have a UI.

User interface changes

None

API changes

None

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Instructions
Reroll the patch if it no longer applies. Instructions
Update the issue summary Instructions
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards Instructions
Files: 
CommentFileSizeAuthor
#15 interdiff-14-15.txt642 bytesdinarcon
#15 tidy_up_gz_conf-2183075-15.patch2.72 KBdinarcon
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,667 pass(es). View
#14 tidy_up_gz_conf-2183075-14.patch2.03 KBer.pushpinderrana
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,586 pass(es). View
#12 tidy_up_gz_conf-2183075-12.patch2.03 KBamitgoyal
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,225 pass(es). View

Comments

valderama’s picture

FileSize
2.65 KB
PASSED: [[SimpleTest]]: [MySQL] 63,563 pass(es). View

Here is a first patch, which reverses the order of the extension_load() and the config() calls and removes the section in the default.settings.php

Is the patch fine like this?

valderama’s picture

Status: Active » Needs review
valderama’s picture

FileSize
2.65 KB
PASSED: [[SimpleTest]]: [MySQL] 63,589 pass(es). View
valderama’s picture

Removed the whitespace in the if-condition...

lucastockmann’s picture

Maybe a side Issue, or could be fixed here:
drupal_page_set_cache() has wrongly documented params.

valderama’s picture

The documentation change belongs into a new issue. Should be tagged with "Documentation". And maybe check if there is an issue about this change already.

umar-ahmad’s picture

FileSize
2.67 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch tidy_up_gz_conf-2183075-7.patch. Unable to apply patch. See the log in the details link for more information. View

Rerolled the earlier patch

cilefen’s picture

7: tidy_up_gz_conf-2183075-7.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 7: tidy_up_gz_conf-2183075-7.patch, failed testing.

cilefen’s picture

dinarcon’s picture

Assigned: Unassigned » dinarcon

Working on this.

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
2.03 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,225 pass(es). View

Please review revised patch as the patch in #7 no longer applies.

cilefen’s picture

Issue tags: +Needs reroll
er.pushpinderrana’s picture

Issue tags: -Needs reroll
FileSize
2.03 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,586 pass(es). View

Rerolled above #12 patch.

dinarcon’s picture

Assigned: dinarcon » Unassigned
FileSize
2.72 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,667 pass(es). View
642 bytes

The issue summary states that the condition swap should also be made in drupal_page_set_cache() This patch adds to @er.pushpinderrana's patch in #14 to account for that.

znerol’s picture

Status: Needs review » Reviewed & tested by the community

Thanks. Patch still applies and it does address the requirements listed in the issue summary.

  • catch committed a255181 on 8.0.x
    Issue #2183075 by valderama, dinarcon, er.pushpinderrana, amitgoyal,...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.