Hello guys,
After doing some reviews lately.. I saw some contributors using hook_help() getting the existing read me file in the module .. here is an example:
/**
* Implement hook_help().
*/
function dblog_quick_filter_help($path, $arg) {
switch ($path) {
case 'admin/help#dblog_quick_filter':
$output = file_get_contents(drupal_get_path('module', 'dblog_quick_filter') .'/README.txt');
return module_exists('markdown') ? filter_xss_admin(module_invoke('markdown', 'filter', 'process', 0, -1, $output)) : '<pre>'. check_plain($output) .'</pre>';
}
}Which I see is bad not only because its a file and can't be translated NOT using t() ,, its because also lazy and the read me file contents is about everything, its not helpful for the user to see everything NOT what matters in the project as just a summary contains help!
plus if you see the code above you'll notice invoke and filter_xss_admin which I see not necessary from a such a hook!.. in some other module I did a review I saw alot of if() statements!
I just want to know what's your opinions about this! should this be a blocker or at least something major to be mentioned! or its just me :(
Thank you!
Comments
This is not a blocker, and
This is not a blocker, and would not have been as part of the old criteria. In fact, some people consider this good practice, and will do this for the hook_help, and leave more detailed instructions for the module in advanced help topics. Having the README available like this is beneficial for people who install modules via the UI, especially when additional libraries and/or setup is needed (this can supplement hook_requirements())).
I do not think it is worth mentioning. Full disclosure though, I am one people who have been advocating this pattern.
Yes, you are correct that this cannot be translated, but on the other hand maintaining detailed help in code via translatable strings is somewhat unmanageable.
There is an issue somewhere (probably in the infrastructure queue) about using the README as the placeholder module page, which is a parallel to this.
The filter_xss_admin() / check_plain() usage is debatable in retrospect. This isn't user input so isn't deemed unsafe per our safe string handling policy. However, I do not recall the reasoning behind this when people started doing it several years ago (I forget which module I picked up this technique from).
Agreed, I think it is up to
Agreed, I think it is up to the taste of the module author if they want to do this hook_help() approach or not. Should never be an application blocker.
There are several different
There are several different ways to provide documentation on a module, and I think they all serve various purposes and audiences.
Help text standards based on hook_help() are described well here --> https://www.drupal.org/node/632280
README template example is described here --> https://www.drupal.org/docs/develop/documenting-your-project/readme-temp...
The best modules, imo, would utilize both approaches the way they were intended, essentially hook_help() for a quick look at the module basics with links for a deeper dive and then the reads_me for all the nitty gritty needed to install the modules, libraries, dependencies, etc.
I agree read me file in
I agree read me file in hook_help is very helpful for people using UI on their project installs ,, which has its disadvantage with the translation ,,
I think I focused on this because in my area here they always ask ,, is it in Arabic? .. which is important to them even its in the UI ,, as a Drupal community right here translation is important to even chose you approach ,, so I guess as dbt102@gmail.com said ,, best module should go in both approaches ..
I see now its not a blocker ,, but I see it as a major and should be mentioned to cover all cases specially when the module is a big one.
Thank you guys for your opinions regarding this..
What are the current thoughts
What are the current thoughts of project reviewers about this? I like the approach of using README in hook_help because it can reduce duplication of content.
PAReview actually lists the link on Drupal.org where this approach is listed out when it finds that hook_help is not implemented in code.
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...
"Be the change you wish to see in the world", M. K. Gandhi.