Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Background:
This issue is part of the task to update/create the hook_help texts of the modules for Drupal 8:
#1908570: [meta] Update or create hook_help() texts for D8 core modules
Tasks:
- write the hook_help function
- review d.o. docs at http://drupal.org/documentation/modules/menu_link
Comment | File | Size | Author |
---|---|---|---|
#24 | create-help-text-menu_link-2033413-24.patch | 1.04 KB | batigolix |
#23 | create-help-text-menu_link-2033413-23.patch | 1.04 KB | batigolix |
#16 | create-help-text-menu_link-2033413-16.patch | 849 bytes | amateescu |
#10 | create-help-text-menu_link-2033413-10.patch | 850 bytes | batigolix |
#6 | create-help-text-menu_link-2033413-6.patch | 849 bytes | batigolix |
Comments
Comment #1
jhodgdonSince there is currently no help for this module, in violation of the Documentation Gate, this is a critical issue.
Comment #2
batigolixFirst attempt
Comment #4
batigolix#2: create-help-text-menu_link-2033413-2.patch queued for re-testing.
Comment #5
jhodgdonI think that this help text is fine. However, I am not sure about the links:
a) You need to use the url() function to make internal links.
b) I don't think linking to admin/structure/menu with link text "Menu module" is a good idea. Normally within help we just link to the other module's help page, not the configure page. So probably we just need one link to the Menu module's help.
c) Also, modules are proper names so "Menu Link module" is the right name, not "Menu link module".
Comment #6
batigolixattached patch:
- fixes a), b), c) from #5
i didnt know about using the url function for links. i probably made this mistake in plenty of other hook help texts as well...
Comment #8
jhodgdon#6: create-help-text-menu_link-2033413-6.patch queued for re-testing.
Comment #9
jhodgdonThanks!
"It is required by the Menu module which provides an interface for managing menus." ==> needs a comma before "which".
Other than that, This text looks good to me.
I haven't seen any other url() problems but I'll keep an eye out. You only need url() for doing within-site links, not external links to drupal.org etc.
Comment #10
batigolixpatch adds comma
Comment #11
jhodgdonLooks good to me! Can someone who didn't write the patch please install and test? Check that the formatting is fine and that the link works. Then mark it "RTBC" if all is well. Thanks!
Comment #12
amateescu CreditAttribution: amateescu commentedTested manually and all is well :)
Comment #13
catchTrailing whitespace here.
Comment #14
amateescu CreditAttribution: amateescu commentedI let that slide because we have
git apply --whitespace=fix
:)Comment #15
jhodgdonThat tool may exist, but that doesn't mean all maintainers use it when applying patches. I usually use the "patch" command to apply patches for docs, not git apply.
Comment #16
amateescu CreditAttribution: amateescu commentedOkay, let's not wait for a whitespace then :)
Comment #17
catchCommitted/pushed to 8.x, thanks!
Comment #18
ParisLiakos CreditAttribution: ParisLiakos commentedwe need a followup for this
Comment #19
catchFixed locally and pushed. I usually spot those but apparently not this time...
Comment #20
ParisLiakos CreditAttribution: ParisLiakos commentedawesomeness, thanks!
Comment #22
lostkangaroo CreditAttribution: lostkangaroo commentedNeeds a reroll with updated links
Comment #23
batigolixPatch with updated links
Comment #24
batigolixnew patch that changes to @link to !link
Comment #25
jhodgdonShould be fine... Can someone manually test to verify the link works in the latest Drupal 8?
Comment #26
jhodgdontag
Comment #27
lostkangaroo CreditAttribution: lostkangaroo commentedManually Tested and works as intended. Two Thumbs up.
Comment #28
webchickCommitted and pushed to 8.x. Thanks!