This blocks #1712250: Convert theme settings to configuration system

Problem

  • The configuration system does not support themes, at all.
CommentFileSizeAuthor
#5 drupal8.config-theme-followup.patch1.34 KBeffulgentsia
#1 drupal8.config-theme.1.patch3.03 KBsun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Needs review
Issue tags: +API change
StatusFileSize
new drupal8.config-theme.1.patch3.03 KB

Attached patch enables installation of default config for themes.

Current HEAD does not uninstall configuration for themes at any time, and I have a (rather critical) follow-up change to the uninstallation of configuration for modules in #1626584: Combine configuration system changes to verify they are compatible, so I'd like to work on proper uninstallation of config in a separate issue.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

Looks good to my eyes, a straightforward but necessary change. Good catch.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Looks good to me "three". Committed to 8.x.

Status: Fixed » Closed (fixed)

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

effulgentsia’s picture

effulgentsia commented
Status: Closed (fixed) » Needs review
StatusFileSize
new drupal8.config-theme-followup.patch1.34 KB

Does this follow-up patch make sense? Discovered by attiks while working on #1775774: Allow themes to identify their breakpoints to Drupal.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Both hunks make sense to me, but I'd really prefer to get #1067408: Themes do not have an installation status in first.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

Looks good.

attiks’s picture

attiks commented

#6 Can't we get this committed first, it's a very small patch?

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks!

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