When the Menu module was renamed "Menu Ui" ... This was bad:
a) The hook_help() wasn't updated. There are probably other places in the code/docs where it is still called "the Menu module" instead of its new name, such as in other hook_help text output in other modules that refer to the menu module.
b) "Ui" is not a good name. It should be "UI" in the info.yml file. UI is an acronym for User Interface, not a word that should have only the first word capitalized.
Fixing this is probably a good Novice task? The task is: Find all the places in the code where it refers to "the Menu module" and change them to "the Menu UI module". And also fix the menu_ui.info.yml file to use the correct capitalization.
This should be done ASAP since it is a UI text issue and affects translation.
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff-10-12.txt | 5.04 KB | cs_shadow |
#12 | drupal-menu_to_menu_ui-2243549-12.patch | 17.66 KB | cs_shadow |
#10 | menu_to_menuui_text_changes_3_2243549.patch | 14.4 KB | agerson |
Comments
Comment #1
tim.plunkett"no good :("?
Comment #2
agerson CreditAttribution: agerson commentedWe do run into some awkward comments like changeing "Verifies a menu link using the menu module UI." to "Verifies a menu link using the menu UI module UI." Should it be "Verifies a menu link using the UI" or "Verifies a menu link using the menu UI module"?
Comment #3
dawehner:( Well, actually the help text should not be required to change as the human readable name of the module did not changed. Maybe this is an oversimplification.
Comment #4
jhodgdon#3 is incorrect. The human-readable name of the module changed from Menu to Menu UI in the yml file.
#2 - if this is in a code doc comment, then you can do something like "Verifies a menu link using the UI of menu_ui.module". Or this would also be a better wording: "Verifies a menu link using the UI of the Menu UI module.".
Note: When referring to the human-readable module, it should always be:
"the Menu UI module"
with that exact capitalization.
Comment #5
agerson CreditAttribution: agerson commentedOk. How does this look:
Comment #6
agerson CreditAttribution: agerson commentedComment #7
agerson CreditAttribution: agerson commentedMissed one important one!
Comment #8
agerson CreditAttribution: agerson commentedComment #9
dcrocks CreditAttribution: dcrocks commentedI count 28 occurrences of the string 'menu module' in a current D8 install.
Comment #10
agerson CreditAttribution: agerson commentedUpdated the patch to include files outside of the Menu UI module folder
Comment #11
jhodgdonThanks -- this is looking pretty good!
A few suggestions:
a) MenuTest.php:
How about just "... and delete them using the UI"? (My logic: this is a test in the Menu UI module, so I don't think we necessarily need to explicitly say we are testing the UI of the Menu UI module, as that would be assumed?) (and this could be similar for the other methods in this file)
b) I am certain that this does not cover all of the spots in other modules that refer to the old name of the module. For instance, the function menu_link_help() mentions the Menu module (should be Menu UI now). contextual.module help also has a reference. There may be others. Try something like this:
Comment #12
cs_shadow CreditAttribution: cs_shadow commentedAttached is a patch that adds the suggestions in #11 to the patch in #10. Most of the tests are now renamed to something like '...using the UI' instead of '...using the UI of the Menu UI module'.
Also, I've tried to find and replace all occurrences of 'menu module' with 'menu ui module'. Let me know if I missed out something.
Comment #13
jhodgdonThanks! I think the patch includes everything it should now, and it all looks good.
By the way, we're in a "criticals and majors only" pre-alpha period, so this cannot be committed for a few days.
Comment #14
alexpottCommitted 3937bda and pushed to 8.x. Thanks!