On #2148255: [meta] Make better D8 api.d.o landing page, linked to high-level overview topics, and put it in Core api.php files, we made a patch that included a stub Topic page for api.drupal.org (i.e., a @defgroup) titled:
Views overview
This can be found in file core/modules/system/core.api.php where it says
@defgroup views_overview
The documentation to go on this page needs to be written. The idea is:
a) Write a few paragraphs about the topic.
b) Link to more detailed documentation on
https://drupal.org/developing/api/8
c) If the more detailed documentation does not yet exist, create stub page(s), link to the stub pages, and add a note to this issue stating that the stub pages need to be filled out.
d) If the topic has related classes, interfaces, and functions, add
@ingroup views_overview
to their documentation headers. That will make these classes etc. show up on the Topic page on api.drupal.org.
For more info -- documentation standards for @defgroup/@ingroup:
https://drupal.org/coding-standards/docs#defgroup
Comment | File | Size | Author |
---|---|---|---|
#11 | 2216539-views-docs-fixup.patch | 4.11 KB | jhodgdon |
Comments
Comment #1
jhodgdonSince this was filed, quite a bit of Views API docs have appeared on api.drupal.org.... will figure out how to integrate it.
Comment #2
jhodgdonWell. This ended up being a "down the rabbit hole" issue.
There were a lot of topics for Views already... The objective here is that developers would be able to have links to information they'd need in order to interact with the Views module API. So... Here is what I did in this patch:
1. We had put in a skeleton Views Overview topic, linked from the main api.drupal.org D8 landing page (that was the subject of this issue). I moved this into the views.api.php file, since that seemed a better home than core.api.php in the system module. I also filled it out with links to other topics and hooks.
2. I looked through the list of existing Views topics on api.drupal.org. There was on on Ajax that seemed like it was only documenting some internals of how Views does Ajax. So I removed this topic -- the purpose of topics is for developers to discover APIs, not to document internals of modules, so it seemed like we did not need this one.
3. The views.api.php file had a plugin overview topic, which attempted to list all the types of plugins. But it was incomplete... so what I did was remove all that documentation there and put it onto the individual plugin type topics instead. Then each of those plugin type topics was given @ingroup views_plugins so that it would (a) show up on the Views Plugins overview page (it will list topics that are @ingroup, just like it will list functions and classes), and (b) each individual type topic would have a "related topics" link back to the overview page.
4. A few plugin types did not have topics. I added topics for them.
5. Developers of plugins need to know: the annotation class, the plugin base class and/or interface, and the plugin namespace. So I put this information into each plugin type topic.
6. Developers go to the list of all Annotations to discover types of plugins. So annotations need to link to the documentation listed in #5. So, I added @ingroup (specific plugin topic) to each of the annotations. This covers
#2288517: Views annotation classes need @ingroup added, which has now been marked as a duplicate of this issue.
7. I also added a @see to the Plugin API topic to each of the specific plugin type topics.
8. I did a little bit of additional standards cleanup and clarity/typo cleanup in some docs headers (hard to resist). Mostly just the @file headers (Definition of vs. Contains), and making all the topics have similar layout/style/wording. As one note on that... The Field and Filter plugins had some documentation on what you can put into hook_views_data(). In one it was on the base class, and in the other it was on the topic. I moved them both to the topic. Also, one of the plugin base classes had a bunch of information on how to do annotations; I took that out as we have centralized docs on this now (or will as soon as a patch is committed).
Whew! I think that is all my notes.... And I think after this patch, plus or minus any typos or bits of misinformation that hopefully the reviewer(s) will find, we'll have considerably better Views documentation for developers. There is more that could be done that I noticed ... but at least developers should be able to get what they need. I hope.
Comment #3
dawehneri wonder who moved that in, given that this probably did not existed in views previously
i am a bit confused about these @ingroup annotations ... i thought they existed to list all of those handler types
Is there a standard to remove references to base classes?
<3
Really nice
What about "does nothing."? This makes it a little clearer for me.
Should we mention pagers as well?
Note: click sortable is the default now
Did you found an issue to rename that variable?
Comment #4
jhodgdonThanks for the review in comment #3!
Regarding point 1 -- we created the views overview in another issue, when we created the new and improved api.drupal.org landing page for Drupal 8. Lots of "stub" topics were added to core.api.php, including this one.
Regarding point 2 -- The @ingroup did not exist in the Annotation classes. Annotation classes are an important way for people to discover plugin types, so they need to be linked to relevant other classes and documentation. Since Views provides this documentation in the "Plugin type foo" topics, the Annotation classes need to have these @ingroup lines to link to those topics. See #2269389: [meta] Make sure plugin developer info is discoverable for more information on this subject.
Regarding point 3 and point 4 - no, in fact just the opposite. See above issue. Interfaces, base classes, plugin manager classes, and annotations (if they exist) all need to be linked together so that plugin developers can find them. Remember, all of this topic/@see/@ingroup documentation is for developers who need to be able to find information on how they can create plugins.
Regarding points 6, 7, 8: Addressed.
Regarding point 9, it looks like $no_operator and $always_required variables should both be renamed. Are you saying there is already an issue? Hopefully it will also fix the docs when/if it happens.
Other changes in this patch:
- I rerolled the patch -- views.api.php segment didn't apply -- this reroll happened before the interdiff file.
- I found an error in the Exposed plugins docs, fixed that.
- I just noticed the "views_objects" topic, which contains only one class. This seems pointless. There are no references to this @defgroup anywhere, and we already have a LOT of topics on api.drupal.org. So I removed it, and instead added a link in the Views Overview topic to this one class.
Comment #5
dawehnerThank you. I don't really have time to review that in more detail but for sure, this is already a huge step forward.
Comment #6
webchickWow, overhaul is right. :) These changes all look good to me. A little sad to be losing that demo_pager code, but I can understand not wanting to do that as a one-off.
Committed and pushed to 8.x. Great work!
Comment #8
damiankloip CreditAttribution: damiankloip commentedThanks for doing this, sorry I didn't review sooner. Daniel did admit he didn't really look though.... I spent a bit of time looking through:
The actual class is not relevant nowadays, as you don't need to
use
annotation classes anymore. Maybe just "They must be annotated with @ViewsAccess" ?This looks strange, it's not a namespace directory. I know what you mean though. I think just namespace is good here.
Area handlers have to implement a render() method at the very least.
Should we be removing the group for this base class?
Here has a group.
Implementations for these should really be providing a getArgument() method.
"This can be useful..", otherwise I think it makes it sound like this is the main use case.
can we stick to arguments and then (contextual filters) as that is how it evolved in other places in the patch.
Also, these should implement a validateArgument() method.
"loading of caches in views" - huh? Loading of cached data maybe?
Also, they don't really control the storage. This is delegated to the cache system. You could do this yourself if you override everything in the base class. Not too likely though.
This can be in the form
identifier => fieldname
too - which is the most common case.control how
Comment #9
damiankloip CreditAttribution: damiankloip commentedComment #10
jhodgdonThanks for looking at this more carefully in #8!
Regarding these comments:
(1) and (2) are how we are doing this for other plugins outside of Views, so I would prefer to leave it. The annotation reference needs to give the full namespace of the annotation class so people can find it, and the namespace given is *not* the full namespace, but a subdirectory under the implementing module's namespace.
(3)/(6) We can mention particular methods that have to be done, but ... aren't they already abstract on the base class? That would be the best way to clue people in to "you must implement this", not to add to this topic docs.
(4) It is inside a @defgroup @{ @} so the extra @ingroup is not needed.
(5) isn't inside a @defgroup @{ @} so it needs the @ingroup.
#7 I can fix.
#8... Really this is very confusing. I realize that prior to Views 7.x, they were called "arguments", but they have been "contextual filters" in the UI for some time now. I think you need to really consider getting rid of calling them "arguments" anywhere in the code, because you are just going to confuse people. Reversing the references in this doc will not help. Let's open a different issue to address this.
#9, #10, #11 I can fix.
I'll make a new patch shortly...
Comment #11
jhodgdonDecided you were right on #8 too. Here's a patch.
Comment #12
webchickLooks like all feedback's been addressed.
Committed and pushed to 8.x. Thanks!