Add a dedicated @EntityType annotation

Yes! This looks great.

Does this hard-code what properties the annotation supports, and can no longer be extended by contrib?

Well this seems like an entirely sensible idea.

I'm never going to disagree with more explictness in what you mean, and I *hate* @Translation from the very pit of my soul. This also implies we could actually document what the hell properties an @EntityType has, which was one of quicksketch's main concerns when he first hit annotations.

The advantage of @Translation is that you can scan actually the list of available strings.

Why do you care, if magical fairy elves take care of it automatically for you? :)

Damn. I knew it was too magical to be true.

+        $values[$key] = t($values[$key]);

Channeling Gábor from #1849610-36: Improve "add" links accessibility:

Problem is your approach with $whatever = 'boo'; t($whatever) makes 'boo' translatable only when this code runs. When the code is not run, there is *no way* to tell what is in $whatever when t($whatever) is looked at. Reality is modules cannot be *run* to find their translatable strings when localize.drupal.org collects translatable strings. ... So while $whatever = 'boo'; t($whatever) makes 'boo' translatable *on the site*, there is no community pre-translation possible. The UI can only be translated *after the fact*, *only on the site itself* once at least one user faced the untranslated UI at least once (so the Drupal runtime figures out the string to translate dynamically and puts it into the database).

I don't think we can kill @Translation, sadly. :\ Though why it is @Translation and not @t is beyond me (and not for this issue).

#1933984: Support for Drupal 8 @Translation annotations is not yet solved in potx, but I feel it would be orders of magnitude easier to parse our strings wrapped in @Translation vs. trying to identify what kind of annotation this is and then what keys should be saved for translation from that kind of annotation (with static code analysis).

I would also point out it's a little bit of a DX-WTF if I only have to wrap certain strings in @Translation but others I shouldn't because it's taken care of for me. So me writing a new plugin, how do I know which should I use? I would rather we suffer and just have @Translation defined in each annotation rather than trying to make it automatic, and the job of potx harder.

+++ b/core/lib/Drupal/Core/Entity/Annotation/EntityType.phpundefined
@@ -0,0 +1,255 @@
+class EntityType extends Plugin {

Just wondering whether all controllers should be defined in the same place in this class.

This is so many flavors of awesome that we should make a sundae bar.

+++ b/core/lib/Drupal/Component/Annotation/Plugin.phpundefined
@@ -34,7 +36,13 @@ class Plugin implements AnnotationInterface {
-    $this->definition = $this->parse($values);
+    $reflection = new \ReflectionClass($this);
+    // Only keep actual default values by ignoring NULL values.
+    $defaults = array_filter($reflection->getDefaultProperties(), function ($value) {
+      return $value !== NULL;
+    });
+    $parsed_values = $this->parse($values);
+    $this->definition = NestedArray::mergeDeep($defaults, $parsed_values);

As discussed on IRC, we could also use get_object_vars($this) here, but well, that's just an option.

+++ b/core/lib/Drupal/Core/Entity/Annotation/EntityType.phpundefined
@@ -0,0 +1,262 @@
+ * Contains Drupal\Core\Entity\Annotation\EntityType.

"\"

+++ b/core/lib/Drupal/Core/Entity/EntityManager.phpundefined
@@ -171,7 +47,10 @@ class EntityManager extends PluginManagerBase {
-    $this->discovery = new AnnotatedClassDiscovery('Core', 'Entity', $namespaces);
+    $annotation_namespaces = array(
+      'Drupal\Core\Entity\Annotation' => DRUPAL_ROOT . '/core/lib',
+    );
+    $this->discovery = new AnnotatedClassDiscovery('Core', 'Entity', $namespaces, $annotation_namespaces, 'Drupal\Core\Entity\Annotation\EntityType');

It's great that we don't need another discovery class, as this would be confusing for some people.

+++ b/core/lib/Drupal/Core/Entity/Annotation/EntityType.phpundefined
@@ -0,0 +1,262 @@
+ * Contains Drupal\Core\Entity\Annotation\EntityType.

"\"

This looks like a HUGE improvement to my eyes. RTBC!

Eclipse

Just in general I'm wondering whether/how we should document that a specific value is optional (for example render_controller_class).

When we document parameters on hook_foo_info() and the like, we just prefix their description with "(optional) ".. we should probably do that here as well, so we don't lose that information.

If it comes back green, I say RTBC.

Eclipse

♥♥♥

F*ck yeah! This looks great!!

Committed and pushed to 8.x! :D Yay for a big step in the right direction of mere mortals being able to grok D8. :)

Minor : unless I'm mistaken, If EntityManager has no more $defaults, it doesn't need ProcessDecorator() anymore ?

Doesn't this need change notice updates?