Quoting @jhodgdon from #1968970-13: Standardize module-provided entity info documentation and clean-up the @EntityType annotation [wrt to the docblock of EntityTranslationControllerInterface]:
- How about a @defgroup so it shows up as a Topic?
- This @file doc starts out "The entity translation UI relies on the entity info to provide its features." and then doesn't explain what "entity info" is or how to define it (a hook? plugin annotation?).
- Then the next paragraph starts out telling me that some keys are required but not all, and then gets really really confusing so that I can't even follow it. I think it would be better to just make a list of the keys (using our standard list formatting to indicate (optional) for optional keys and explaining for each key why/when it is really optional).
- The rest of the documentation in the @file just seems like a ramble... it really needs to be organized.
So... Can the documentation be rewritten so that it
a) Is in a @defgroup not @file, with relevant classes/hooks/whatever using @ingroup so they reference each other?
b) Actually tells a developer what they need to do to define a translatable entity class.
c) Is in a logical order.
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #1
plach@jhodgdon:
That docblock has been considerably reduced (and moved to the
content_translation_entity_info_alter()PHP docs), since this issue was open: would you please give it another look before we start any work here?Comment #2
jhodgdonIMO, putting all of that information into content_translation_entity_info_alter() is not useful at all. How would someone know where to find it? My main concern above still stands -- the docs need to be in a @defgroup and not in some obscure location.
And I also still think the docs need to be rewritten. They are kind of just a jumble of thoughts in no particularly logical order. Who is the audience for this documentation? What would a logical title be? What does this audience need to know?
So I think most of what I said earlier still applies.
Comment #3
jhodgdonComment #4
plach@jhodgdon:
In #1968970-18: Standardize module-provided entity info documentation and clean-up the @EntityType annotation you suggested to move them there:
I think that place makes sense as the whole docblock describes entity info keys. If you don't agree I guess we need to revert #1968970: Standardize module-provided entity info documentation and clean-up the @EntityType annotation as this issue was only meant to improve the doc content.
Comment #5
jhodgdonOK then. For this issue, let's ignore the "how will people discover this" piece and at least rewrite the documentation so that it makes sense.
Comment #6
jhodgdonAh. And I see that on that other issue's patch, some documentation pointers were made to hook_entity_info() and hook_entity_info_alter(), so people should know to look there. Should be fine.
Comment #7
jhodgdonComment #8
plachAwesome, I will try to put something together later :)
Comment #9
plachHere is a first attempt: CT entity info have been simplified quite a bit since that docblock was originally written, so there is way less stuff to document :)
I omitted the
'translation'key as it is obsolete and it will be removed ASAP.Comment #10
jhodgdonYes, does seem to be simpler -- DX->simple() == good.
So... I think this is pretty good, but it could be improved:
a) The documentation starts out by saying that the entity info needs two entity info items and then goes on to say in the details of each one that they're actually optional. I found it a bit confusing.
b) The second item refers to "link templates" but in the entity info, I think these are actually in the "links" section. I think it's probably better to use the actual element name?
c) For more novice developers, it might be good to mention annotation in the first paragraph explicitly? Maybe rather than saying "needs two entity info items" it would be better to say something like "references two elements from the EntityType annotation in the entity class documentation header"? It's a few more words but might save some head scratching.
d) It's obvious if you read the code in this info_alter function, but maybe it should be mentioned that the entity type's isTranslatable() method needs to return TRUE... and I personally am not sure how to make sure that happens (maybe it's that the "translatable" element in the EntityType is set to TRUE?)... maybe it would be a good idea to explain that too? or how to make it not happen, if for some reason you do not want your entity to be translatable?
Comment #11
jhodgdonBy the way, and maybe this should be back on the other issue... Where are all of the things that go into @EntityType actually documented anyway?
I went to
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Entity!EntityType... (the annotation class, I think, for @EntityType annotation?)
and there is not a link to hook_entity_info_alter() and there isn't much at all there in that annotation class. How again is someone supposed to discover where to discover all the @EntityType elements?
Comment #23
darvanenTraiged as part of the Bug Smash Initiative (#bugsmash on slack). Determined to still be a bug due to misleading documentation.
Needs a reroll, and the IS needs the template applied and filled out.
Comment #24
immaculatexavier commentedRerolled patch against #9
Comment #25
medha kumariReroll the patch #24 with Drupal 9.5.x
Comment #26
medha kumariReroll the patch #24 with Drupal 10.1.x
Comment #27
smustgrave commentedFor issue summary update.
Comment #31
quietone commentedLets see if we can complete this.
Comment #32
smustgrave commentedSo looking at the quote of the summary talks about adding @defgroup but that's not the case. So is the goal just to rewrite it now?