It would be useful if the module had an informative section as usual, in the form of basic hook_help.

CommentFileSizeAuthor
#7 needs-hook-help-3046659-7.patch2.47 KBdavidjguru
#5 needs-hook-help-3046659-5.patch2.42 KBdavidjguru
#2 needs-hook-help-3046659-2.patch3.2 KBdavidjguru

Comments

davidjguru created an issue. See original summary.

davidjguru’s picture

davidjguru
he/him
Primary language Spanish
Location Seville, Andalusia (Spain)
commented
StatusFileSize
new needs-hook-help-3046659-2.patch3.2 KB

I just made a small patch with the hook help for this module.

davidjguru’s picture

davidjguru
he/him
Primary language Spanish
Location Seville, Andalusia (Spain)
commented
Status: Active » Needs review
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Component: Code » Documentation
Category: Bug report » Task
Status: Needs review » Needs work

Here's some quick review, I think we would want to make it smaller and not to have too much verbosity in this hook.

  1. +++ b/humanstxt.module
@@ -5,6 +5,60 @@
+    $output = '';

    The $output needs to be indented once more 

  2. +++ b/humanstxt.module
@@ -5,6 +5,60 @@
+                          project <a href="http://humanstxt.org/"

    I think that the convention to add href to translated texts is to use wildcards, see ban_help in core for an example. 

  3. +++ b/humanstxt.module
@@ -5,6 +5,60 @@
+                          target="_blank">Humans.txt</a>. You can see an

    Let's not decide to open the link in a blank, the user should be free to do so 

  4. +++ b/humanstxt.module
@@ -5,6 +5,60 @@
+    $output .= '<p>' . t('<a href="http://cambrico.net/contact" target="_blank">

    I don't think this should be in the help of the module :)

davidjguru’s picture

davidjguru
he/him
Primary language Spanish
Location Seville, Andalusia (Spain)
commented
StatusFileSize
new needs-hook-help-3046659-5.patch2.42 KB

Thanks for the feedback!
As suggested, the output has been indented, the links are already using wildcards and I have removed target = "_ blank" in the links.
I have also removed unnecessary external links and I have reduced the verbosity of the help text.
I attach a new revised patch.
Greetings.

davidjguru’s picture

davidjguru
he/him
Primary language Spanish
Location Seville, Andalusia (Spain)
commented
Status: Needs work » Needs review
davidjguru’s picture

davidjguru
he/him
Primary language Spanish
Location Seville, Andalusia (Spain)
commented
StatusFileSize
new needs-hook-help-3046659-7.patch2.47 KB

I add a new version with the phpcs annotations already fulfilled.

  • pcambra committed 1fa23ae on 8.x-1.x authored by davidjguru 
    Issue #3046659 by davidjguru, pcambra: The module needs a basic hook...
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Status: Needs review » Fixed

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.