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

Files: 
CommentFileSizeAuthor
#24 create-help-text-menu_link-2033413-24.patch1.04 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 58,499 pass(es).
[ View ]
#23 create-help-text-menu_link-2033413-23.patch1.04 KBbatigolix
PASSED: [[SimpleTest]]: [MySQL] 58,918 pass(es).
[ View ]
#16 create-help-text-menu_link-2033413-16.patch849 bytesamateescu
PASSED: [[SimpleTest]]: [MySQL] 57,158 pass(es).
[ View ]
#10 create-help-text-menu_link-2033413-10.patch850 bytesbatigolix
PASSED: [[SimpleTest]]: [MySQL] 56,637 pass(es).
[ View ]
#6 create-help-text-menu_link-2033413-6.patch849 bytesbatigolix
PASSED: [[SimpleTest]]: [MySQL] 56,994 pass(es).
[ View ]
#2 create-help-text-menu_link-2033413-2.patch900 bytesbatigolix
PASSED: [[SimpleTest]]: [MySQL] 56,524 pass(es).
[ View ]

Comments

Priority:Normal» Critical

Since there is currently no help for this module, in violation of the Documentation Gate, this is a critical issue.

Status:Active» Needs review
StatusFileSize
new900 bytes
PASSED: [[SimpleTest]]: [MySQL] 56,524 pass(es).
[ View ]

First attempt

Status:Needs review» Needs work

The last submitted patch, create-help-text-menu_link-2033413-2.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

I think that this help text is fine. However, I am not sure about the links:
'/admin/help/menu','@menu' => '/admin/structure/menu'))

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".

Status:Needs work» Needs review
StatusFileSize
new849 bytes
PASSED: [[SimpleTest]]: [MySQL] 56,994 pass(es).
[ View ]

attached 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...

Status:Needs review» Needs work

The last submitted patch, create-help-text-menu_link-2033413-6.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

Thanks!

"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.

Status:Needs work» Needs review
StatusFileSize
new850 bytes
PASSED: [[SimpleTest]]: [MySQL] 56,637 pass(es).
[ View ]

patch adds comma

Issue tags:+Needs manual testing

Looks 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!

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs manual testing

Tested manually and all is well :)

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/menu_link/menu_link.moduleundefined
@@ -7,6 +7,16 @@
+      $output .= '<h3>' . t('About') . '</h3>';
+      $output .= '<p>' . t('The Menu Link module allows users to create menu links. It is required by the Menu module, which provides an interface for managing menus. See the <a href="@menu-help">Menu module help page</a> for more information.', array('@menu-help' => url('admin/help/menu'))) . '</p>'; ¶
+      return $output;
+  }

Trailing whitespace here.

I let that slide because we have git apply --whitespace=fix :)

That 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.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new849 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,158 pass(es).
[ View ]

Okay, let's not wait for a whitespace then :)

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Issue tags:+Needs followup

diff --git a/core/modules/menu_link/menu_link.module b/core/modules/menu_link/menu_link.module
old mode 100644
new mode 100755

we need a followup for this

Fixed locally and pushed. I usually spot those but apparently not this time...

Issue tags:-Needs followup

awesomeness, thanks!

Status:Fixed» Closed (fixed)

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

Priority:Critical» Normal
Status:Closed (fixed)» Needs work

Needs a reroll with updated links

Status:Needs work» Needs review
StatusFileSize
new1.04 KB
PASSED: [[SimpleTest]]: [MySQL] 58,918 pass(es).
[ View ]

Patch with updated links

StatusFileSize
new1.04 KB
PASSED: [[SimpleTest]]: [MySQL] 58,499 pass(es).
[ View ]

new patch that changes to @link to !link

Should be fine... Can someone manually test to verify the link works in the latest Drupal 8?

Issue tags:+Needs manual testing

tag

Status:Needs review» Reviewed & tested by the community

Manually Tested and works as intended. Two Thumbs up.

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 8.x. Thanks!

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