The header documentation is wrong. The original issue #2635784: Deprecate ThemeHandler::setDefault() in favour of configuration and replace usages fixed this in all the tests, and initially commented with $this->config() assuming that TestBase was the class. In comment 40 this was suggested to be \Drupal::config() and that was changed in the comment. The problem is that \Drupal::config() returns the readonly config object whereas TestBase::config() returns the editable config object. We need the editable config object. Attached is a patch that fixes this comment.

CommentFileSizeAuthor
#6 2882784-06.patch745 bytesharsha012
#3 2882784-03.patch731 bytesharsha012
ThemeHandlerInterface-setDefault-deprecated.patch731 bytesdouggreen

Comments

douggreen created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work

Thanks for the patch, the code snippet should end in a semi colon but otherwise this looks fine.

harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2882784-03.patch731 bytes

fixed the issue

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Thanks!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/lib/Drupal/Core/Extension/ThemeHandlerInterface.php
@@ -167,7 +167,13 @@ public function getDefault();
+   * Use
+   * @code
+   *   \Drupal::configFactory()
+   *     ->getEditable('system.theme')
+   *     ->set('default', $theme)
+   *     ->save();
+   * @endcode

Since this is part of the @deprecated section, this should be intended with 2 more spaces

harsha012’s picture

harsha012 commented
Status: Needs work » Needs review
StatusFileSize
new 2882784-06.patch745 bytes

fixed the indent issues

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Thanks for the quick fix. Didn't know that about @deprecated, thanks @lauriii

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

  • lauriii committed 40f9a70 on 8.5.x 
    Issue #2882784 by harsha012, douggreen, joelpittet: Fix...
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Version: 8.5.x-dev » 8.4.x-dev

Committed 40f9a70 and pushed to 8.5.x. Thanks!

Leaving as RTBC against 8.4.x

  • xjm committed b73952b on 8.4.x authored by lauriii 
    Issue #2882784 by harsha012, douggreen, joelpittet: Fix...
xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Cherry-picked back to 8.4.x as well; thanks @lauriii.

Status: Fixed » Closed (fixed)

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