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.
part of #2011290: [meta] standards cleanup to get ready for getting into core
Clean up the following file:
http://drupalcode.org/project/config_translation.git/blob/HEAD:/lib/Drup...
Proposed resolution
Remaining tasks
User interface changes
No
API changes
No
Comment | File | Size | Author |
---|---|---|---|
#4 | config_translation.code_mapperinterface.2017979-4.patch | 7.82 KB | YesCT |
#4 | interdiff-1-4.txt | 6.77 KB | YesCT |
#1 | config_translation.code_mapperinterface.2017979-1.patch | 1.7 KB | YesCT |
Comments
Comment #1
YesCT CreditAttribution: YesCT commented1.
This is called *has* edit tab. But looking at the mappers that implement this interface, it's if it needs and edit tab.
Will needing a edit tab be the same as having an edit tab? Could something need an edit tab, but not have one?
This is probably ok.
2.
Is the menu item type different than the menu type?
What are some examples of menu types or menu item types?
I'm guessing menu types.. are those build in to the default profile, each menu is it's own type.
Then there are also custom menu's those have the type: custom menu type.
I think maybe contrib might be able to add other menu types.
For menu link items, what are their types? Are they the menu's they are in, the bundle? Or is it something more generic?
3.
I'm not sure where to look this up, but when a word is all caps, like ID, how should it be in the method name? Id or ID? I think we ran into that when adding something for a standard time format or some other issue. I did a grep for URL vs Url in function names and there seem to be both...
AH! here it is.
from https://drupal.org/node/608152#naming
So Id is just right.
4.
when should getType return NULL?
5.
This used to say just returns group name. Is the route name usually the same as the group name?
Comment #2
YesCT CreditAttribution: YesCT commentedThe blocking stuff in there was,
a doc block on the class (interface), types on the params, and only @return when returning something.
Other questions are to make sure naming is making sense,
and there were also some typos fixed (which is just nits and not part of the doc core gate, so not blocking).
Comment #3
YesCT CreditAttribution: YesCT commented#1: config_translation.code_mapperinterface.2017979-1.patch queued for re-testing.
Comment #4
YesCT CreditAttribution: YesCT commented1. was really needsEditTab. so changed that.
2. was really getMenuItemType. so changed that.
3. was ok.
4. updated docs to clarify.
5. was ok.
Comment #5
Gábor HojtsyWe reviewed this together with YesCT in Dublin just now. It all looks good, committed!