Book allowed_types settings repetitive and in under certain conditions can change unexpectedly

+++ b/core/modules/book/book.admin.inc
@@ -92,9 +92,14 @@ function book_admin_settings_validate($form, &$form_state) {
 function book_admin_settings_submit($form, &$form_state) {
+  $allowed_types = array_filter($form_state['values']['book_allowed_types']);
...
     // Remove unchecked types.
-    ->set('allowed_types', array_filter($form_state['values']['book_allowed_types']))
+    ->set('allowed_types', $allowed_types)

Stale comment, should be moved up above the new line where unchecked types are removed now.

+++ b/core/modules/book/book.admin.inc
@@ -92,9 +92,14 @@ function book_admin_settings_validate($form, &$form_state) {
+  // We need to save the allowed types in an array ordered by machine_name so
+  // that we can save them in the correct order if node type changes.
+  // @see book_node_type_update().
+  sort($allowed_types);

+++ b/core/modules/book/book.module
@@ -1247,12 +1246,13 @@ function book_node_type_update($type) {
+      // Ensure that the allowed type array is sorted by node type machine name.
+      sort($allowed_types);

The corresponding added test explains this some more, but we should actually clarify why this is done in the functional code, too. Actually, a proper explanation in the functional code is much more important.

In general:

Instead of explaining what the technical code lines are doing à la:

"sort and save values so they appear in the 'correct' order."

Always document the actual whats and whys:

"Ensure that the existing order of enabled book types remains to be identical, (because... | in order to ...)"

Alas, what I'm currently missing is the "why?" — why are we doing this? :)

+++ b/core/lib/Drupal/Core/Config/Context/ConfigContext.phpundefined
@@ -9,31 +9,29 @@
- * A configuration context object provides a data array that can be used as
- * parameters to get customized configuration objects.
+ * A configuration context object provides a data array that can be used:
+ *   - as a parameter to get customized configuration objects.
+ *   - as a store of config data used to override values.

+++ b/core/modules/user/lib/Drupal/user/UserConfigContext.phpundefined
@@ -26,6 +26,14 @@ class UserConfigContext extends ConfigContext {
+  /**
+   * Implements \Drupal\Core\Config\Context\ContextInterface::setUuid().
+   */
+  public function setUuid() {
+    // Use the user's uuid to identify the config context.
+    $this->uuid = $this->get(self::USER_KEY)->uuid();
+  }
+

is this unrelated changes? it's reminding me of #1929136: Fix override-free context, move global config overrides back to an event listener

+++ b/core/lib/Drupal/Core/Config/Context/GlobalConfigContext.phpundefined
@@ -0,0 +1,29 @@
+  }
+}

I think a newline at the end of the class is usual.

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigLocaleOverride.phpundefined
@@ -36,7 +36,7 @@ public function setUp() {
-  /**
+  /*
    * Tests basic locale override.

@@ -44,22 +44,19 @@ function testConfigLocaleOverride() {
-  /**
+  /*
    * Tests locale override based on user's preferred language.

@@ -126,92 +118,30 @@ function testConfigLocaleUserOverride() {
-  /**
+  /*

unintended change I think.

+++ b/core/modules/config/lib/Drupal/config/Tests/ConfigOverrideTest.phpundefined
@@ -101,15 +116,24 @@ function testConfOverride() {
+    // Write file to staging

missing period to end the sentence.

This was just a quick review. I didn't read it really carefully.

Note, this will need a re-roll when the follow-up for #1763640: Introduce config context to make original config and different overrides accessible gets in.

Thanks! :)

Sorry, this is not applying for me...

$ git apply --index 1933548.book-settings.10.patch 
error: patch failed: core/modules/book/lib/Drupal/book/Tests/BookTest.php:405
error: core/modules/book/lib/Drupal/book/Tests/BookTest.php: patch does not apply

adding tag for reroll (also a help document on rerolls: http://drupal.org/patch/reroll)

Thanks! I was trying to remember that tag.

In the hope it comes back green.

Committed and pushed to 8.x. Thanks!