Spin-off from #1867856: Use annotation discovery for data type plugins. We found that the SchemaDiscovery (which we removed in the aforementioned issue and merged it into the TypedConfigManager) should not run the discovery mechanisms in the constructor but instead just invoke that in getDefinitions() when requested. Patch incoming (including some other minor improvements).

Files: 
CommentFileSizeAuthor
#9 interdiff.txt577 bytesfubhy
#9 2030859-9.patch5.16 KBfubhy
PASSED: [[SimpleTest]]: [MySQL] 57,193 pass(es).
[ View ]
#7 2030859-7.patch5.4 KBfubhy
PASSED: [[SimpleTest]]: [MySQL] 57,129 pass(es).
[ View ]
#4 2030859-4.patch5.33 KBfubhy
PASSED: [[SimpleTest]]: [MySQL] 57,206 pass(es).
[ View ]
#1 2030859-1-for-review.patch5.33 KBfubhy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2030859-1-for-review.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 2030859-1-for-testbot.patch78.93 KBfubhy
PASSED: [[SimpleTest]]: [MySQL] 56,840 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new78.93 KB
PASSED: [[SimpleTest]]: [MySQL] 56,840 pass(es).
[ View ]
new5.33 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2030859-1-for-review.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Status:Needs review» Needs work

Agree with the need to fix SchemaDiscovery, but this patch looks a bit too long to me...? (It looks like it includes too #1867856: Use annotation discovery for data type plugins )

Yes, @see the "for-review" patch ;).

Status:Needs work» Needs review
StatusFileSize
new5.33 KB
PASSED: [[SimpleTest]]: [MySQL] 57,206 pass(es).
[ View ]

Re-uploading that patch for testbot :)

Issue tags:+D8MI, +language-config

Looks good to me.

+++ b/core/lib/Drupal/Core/Config/TypedConfigManager.phpundefined
@@ -104,11 +104,11 @@ public function create(array $definition, $value = NULL, $name = NULL, $parent =
+      throw new \InvalidArgumentException(format_string('Invalid data type %plugin_id has been given.', array('%plugin_id' => $plugin_id)));

Actually, that should be String::format() :(

StatusFileSize
new5.4 KB
PASSED: [[SimpleTest]]: [MySQL] 57,129 pass(es).
[ View ]

Yes, this one looks good to me, only one minor issue:

       unset($definition['type']);
-      $this->definitions[$type] = $definition;

Could we keep this last line so we don't need to reprocess the same definition twice and it gets kind of cached, mostly for performance reasons?

StatusFileSize
new5.16 KB
PASSED: [[SimpleTest]]: [MySQL] 57,193 pass(es).
[ View ]
new577 bytes

Sure... Dunno how this affects the tests or why I even removed it. But we'll see! :)

Status:Needs review» Reviewed & tested by the community

Looks good and green :-), thanks, I think this is ready.
(and required for this one to work: #1966538: Translation is not updated for configuration, when the config changes)

Issue tags:+blocker

Tag as blocker being a dependency.

Status:Reviewed & tested by the community» Fixed

Committed d7bbf9f and pushed to 8.x. Thanks!

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