Files: 
CommentFileSizeAuthor
#7 2066145-convert-node_admin_theme-7.patch7.45 KBmtift
PASSED: [[SimpleTest]]: [MySQL] 58,481 pass(es).
[ View ]
#7 interdiff.txt793 bytesmtift
#4 2066145-convert-node_admin_theme-4.patch7.51 KBmtift
PASSED: [[SimpleTest]]: [MySQL] 58,030 pass(es).
[ View ]
#4 interdiff.txt311 bytesmtift
#3 2066145-convert-node_admin_theme-3.patch7.5 KBmtift
PASSED: [[SimpleTest]]: [MySQL] 57,940 pass(es).
[ View ]
#2 2066145-convert-node_admin_theme-2.patch5.64 KBmtift
PASSED: [[SimpleTest]]: [MySQL] 57,874 pass(es).
[ View ]

Comments

Assigned:Unassigned» mtift

Status:Active» Needs review
Issue tags:+Configuration system
StatusFileSize
new5.64 KB
PASSED: [[SimpleTest]]: [MySQL] 57,874 pass(es).
[ View ]

Patch is attached. In cases where we are currently using TRUE (booleans), I converted them to '1'.

StatusFileSize
new7.5 KB
PASSED: [[SimpleTest]]: [MySQL] 57,940 pass(es).
[ View ]

The previous patch was wrong on multiple levels, so please ignore it.

Changing node_admin_them to admin_theme could potentially cause confusion with the admin_theme in the system module (especially in ThemeTest.php). I discussed this with @msonnabaum, who suggested use_admin_theme_for_editing_nodes, which seemed more descriptive, but would more accurately need to be named use_admin_theme_for_editing_or_creating_nodes. That seemed too long. @effulgentsia and I thought use_admin_theme would be sufficiently descriptive so that is what I used.

StatusFileSize
new311 bytes
new7.51 KB
PASSED: [[SimpleTest]]: [MySQL] 58,030 pass(es).
[ View ]

Updated patch to make config in node.settings.yml alphabetical.

Status:Needs review» Reviewed & tested by the community

Actually this looks ok :).

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/node/config/node.settings.ymlundefined
@@ -1 +1,2 @@
+use_admin_theme: '1'

It seems like we're changing the default behaviour here. When it was a variable the default was false and now it true.

+++ b/core/modules/node/node.installundefined
@@ -430,7 +430,7 @@ function node_uninstall() {
+  Drupal::config('node.settings')->delete('use_admin_theme');

There's no need for this line. Default config is automatically removed on uninstall. And delete() does not clear a single key anyway :)

Status:Needs work» Needs review
StatusFileSize
new793 bytes
new7.45 KB
PASSED: [[SimpleTest]]: [MySQL] 58,481 pass(es).
[ View ]

Updated patch attached

Status:Needs review» Reviewed & tested by the community

Back to rtbc unless I'm missing something again?

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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

Issue summary:View changes

Updated issue summary.