Not sure why the methods are abstract ? If my class only needs to react to a couple events handled by the trait, it still needs to explicitly provide empty implementations for all the others. Couldn't the traits provide empty base implementations, with consumer classes only having to override the ones they're actually interested in ?
The phpdocs for the various onEntityType*() / onField*() methods only state {@inheritdoc}, but that's a lie : the there's no parent to inherit a doc from :-)
Also, not sure why the methods are abstract
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | interdiff.txt | 683 bytes | yched |
| #13 | entity-listener_traits-2411791-13.patch | 2.99 KB | yched |
| #1 | entity-listener_traits-2411791-1.patch | 2.32 KB | plach |
Comments
Comment #1
plachWell, actually those traits are supposed to be used by classes implementing
EntityTypeListenerInterfaceand/orFieldStorageDefinitionListenerInterface, which are linked in the doc blocks. That's why those methods are not documented on the traits.Good point about method implementations, anyway.
Comment #4
dawehnerGood trick!
In case you would have just method I'd like it to abstract, because your IDE will tell you, what you need to implement, basically similar to interfaces.
Comment #5
yched commentedOK, so I'm not sure what we RTBC here :-)
- The patch doesn't fix the {@inheritdocs} because, if I read #1 correctly they're kind of "by design" ? (a trait doesn't implement an interface, it is classes that can use the trait to implement some interfaces)
- It removes the "abstract" keywords on methods, which, if I read #4 correctly, was intentional ?
Comment #6
plachYep,
{@inheritdocs}were by design, while methods wereabstractbecause I wrongly assumed that an implementation would need them all.Comment #7
yched commentedOK - updating the issue title and summary accordingly then :-)
Comment #8
alexpottSo can the traits implement the interfaces then?Nope traits can not implement interfacesComment #9
alexpottNot sure what to do about the inheritdoc
Comment #11
plachBot fluke, I guess
Comment #13
yched commentedWe can remove an empty implementation now :-)
Comment #14
plachComment #15
alexpottThe
{@inheritdoc}issue is not created by this patch. And this patch makes it easier to work with the trait. This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 23ced28 and pushed to 8.0.x. Thanks!