Inspired by #1981368: Convert all routes to 'module_name.foo_bar' naming convention.

Updated: Comment #0

Problem/Motivation

Our plugin IDs local_tasks.yml were using _ as a separator. This does not match the new convention decided for routing.yml in #1981368: Convert all routes to 'module_name.foo_bar' naming convention
To be less confusing, use the same convention for local_tasks.yml files.

Proposed resolution

The proposed local_tasks.yml standard is

module_name.sub_name

where sub_name uses lower-case and underscores only.

However, the plugin ID is an arbitrary string so the important element of the standard is using the leading "module_name." to avoid collisions of IDs provided by different modules. The doc page at https://drupal.org/node/2122253 notes that developers may append ".tab" or ".task" if it helps them to remember that the plugin ID is not actually the same as the route name.

Remaining tasks

1. Agree upon the convention.
2. (done) Document the convention as a coding standard. (optimistically done in https://drupal.org/node/2044515/revisions/view/2847981/2850471 since there was no standard for the plugin ID documented there before now.)
3. Patch core to agree with this convention. (there are only 4 local_tasks.yml files, comments, 2 menu_ui tests, views_ui. views_ui is already using this standard)

User interface changes

None.

API changes

?? (update this) Route machine names would change, so if anyone already has alter hooks or any code that depends on specific machine names, that code would break.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

juanolalla’s picture

Status: Active » Needs review
FileSize
5.88 KB

Here is a patch with the conversion.

Status: Needs review » Needs work

The last submitted patch, local-task-routing-convention-2095613-1.patch, failed testing.

g.oechsler’s picture

Fixed to tests. The tests in Menu/LocalTasksTest are now using the changed plugin IDs.

g.oechsler’s picture

Status: Needs work » Needs review

gna

juanolalla’s picture

Status: Needs review » Needs work

Thank you g.oechsler! I didn't have time yesterday to fix it.

juanolalla’s picture

Status: Needs work » Needs review

Sorry, I changed it to need works without knowing it, while you where commenting at the same time.

YesCT’s picture

Status: Needs review » Needs work

ack. this is also changing route names. not just plugin ids.

this is confusing other issues also.
See #2095271: Add default tabs for routes expected by config_translation.

We need another issue to fix route names and take those changes out of this issue.
In the new issue for route names, we need to take out the alter so that it even pays attention to that comment.local_tasks.yml file.

grisendo’s picture

Assigned: Unassigned » grisendo

I take it!

grisendo’s picture

Removed lines from route_name lines

pwolanin’s picture

Status: Needs review » Needs work
Issue tags: -DX (Developer Experience), -Coding standards, -MenuSystemRevamp

The last submitted patch, local-task-routing-convention-2095613-9.patch, failed testing.

grisendo’s picture

Status: Needs work » Needs review
Issue tags: +DX (Developer Experience), +Coding standards, +MenuSystemRevamp
grisendo’s picture

YesCT’s picture

YesCT’s picture

Status: Needs review » Reviewed & tested by the community

looks like we are within scope now.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nice job. :)

Committed and pushed to 8.x. Thanks!

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

pwolanin’s picture

Issue summary: View changes

updated the summary to reference https://drupal.org/node/2122253 and mention that developers may want to append e.g. '.task to the plugin IDs if it helps them keep straight how the different strings are used.

pwolanin’s picture

Issue summary: View changes