Problem/Motivation
Follow up to #1914256: Add all views hooks to hook_hook_info().
All hooks views.api.php in should document in which .inc file they should be placed, see views_hook_info() for the correct locations, either MODULE_NAME.views.inc or MODULE_NAME.views_execution.inc
Steps to reproduce
Proposed resolution
Update all hook documentations in views.api.php to include the correct include file to use. For example do something like:
/**
* Analyze a view to provide warnings about its configuration.
*
* Implementations of this hook should be placed in MODULE_NAME.views.inc.
*
* @param \Drupal\views\ViewExecutable $view
* The view being executed.
*
* @return array
* Array of warning messages built by Analyzer::formatMessage to be displayed
* to the user following analysis of the view.
*/
Remaining tasks
In this issue
- Separate the hooks into two documentation groups: one for the runtime hooks (those invoked from
ViewExecutable, which go in .views_execution.inc) and the others (which go in .views.inc). Add introductory documentation to each group explaining the differences between the two groups and the purpose of separating them (performance). Reference #1914256: Add all views hooks to hook_hook_info()
Followups needed
- Document the field hooks separately
- Deprecate the include file pattern in favor of a different method of on-demand autoloading for the respective hooks
User interface changes
API changes
Data model changes
Release notes snippet
Comments
Comment #4
nikolas haliotis commentedComment #5
smustgrave commentedCC failure
Comment #6
nikolas haliotis commentedComment #7
smustgrave commentedChange matches IS and passes tests now.
Comment #8
quietone commentedComment #9
nikolas haliotis commentedRemoved implementation details for the field hooks and added the implementation details to the missing hooks.
Comment #10
xjmThanks @quietone, I had also gotten hung up on the field hooks when reviewing this. Here's what I had documented aside from that:
query_substitutions: Invoked fromsrc/Plugin/views/query/Sql.phpform_substitutions: Invoked fromsrc/Form/ViewsFormMainForm.phpViewExecutable:views_pre_renderviews_post_renderviews_pre_buildviews_post_buildviews_pre_viewviews_pre_executeviews_post_executeViewExecutable:...which gives us
views_query_alter.Comment #11
xjm@Lendude and I also discussed a followup for this. The use of
.incfiles is archaic, and even in the original issue it was identified that these two magically named include files were bad DX. Their purpose is to avoid loading hundreds/thousands of lines of code that is not needed on a given request, but nowadays we should be able to come up with a better pattern for that with a modern architecture and better DX. To be discussed in the followup. :)Comment #12
xjmA thought: Should the runtime hooks and the non-runtime hooks furthermore be placed in different phpdoc groups? Documenting it on the individual hooks is good for when someone just looks up the individual hook, but if we add two defgroups, that will also give us a spot to document the "why" of the two different hook categories from the original issue.
We should also perhaps have a second followup to document the field hooks differently (with their own docs and in their own third group).
Updating the remaining tasks with the above.
Comment #13
nikolas haliotis commentedSo my understanding is that we want the views.api.php file to be organised like so:
-> a doc block at the start with documentation on the views
-> all the hooks that are relevant
-> a doc block with the documentation for the runtime hooks
-> all the hooks that are relevant
-> a placeholder doc block for the field hooks with a TODO
-> all the field hooks
Also there should be a task about the documentation of the field hooks after this, right?
Comment #14
xjmThanks @Nikolas Haliotis for looking into this. :) Crossposting my remarks from #contribute in Drupal Slack:
Comment #16
dwwGiven that
hook_hook_info()is gone and we're moving to completely kill off procedural .inc files and move all hooks to OOP, I think this should probably be "outdated" at this point. Any objections?Thanks/sorry,
-Derek
Comment #17
quietone commentedI agree, this is most definitely outdated.
Comment #18
dwwOkay, done.
Thanks!
Comment #20
nicxvan commentedJust wanted to give a quick note that all closed issues now get credit and everyone here has been granted credit for helping with this, thank you!