#24 committed.
#28 committed.

Updated: Comment #12

Problem/Motivation

Our route machine names in routing.yml files are not standardized. Some are module_name_sub_name, some are module_name.sub_name, and some may even be following other conventions. It's kind of a mess.

Proposed resolution

Core should standardize on one format, even though it is not technically required by Symfony.

The proposed standard is

module_name.sub_name

where sub_name uses lower-case and underscores only. This appears to be what Symfony CMF does for cases where there is a concept of "module" (Symfony documentation itself does not appear to have a convention like this, as it has no moudles). See comment #6.

Remaining tasks

1. Agree upon the convention.
2. Document the convention as a coding standard.
3. Patch core to agree with this convention.

User interface changes

None.

API changes

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.

(A list of related issues.)

Original report by webchick

If you grep for 'route_name' in 8.x right now, you see a lot of ones that look like this:

./core/modules/language/language.module:    'route_name' => 'language_negotiation_url',

and a few ones that look like this:

./core/modules/views_ui/views_ui.module:    'route_name' => 'views_ui.list',

The easiest thing to do is convert the views ones to under_scores, but I actually prefer the dot-convention myself, because it makes it clear that these are not callback functions, which they kind of look like.

CommentFileSizeAuthor
#28 route-names-1981368-28.patch1.66 KBtim.plunkett
#24 route-names-1981368-23.patch231.63 KBtim.plunkett
#24 interdiff.txt6.24 KBtim.plunkett
#22 route-names-1981368-22.patch225.39 KBtim.plunkett
#22 interdiff.txt7.74 KBtim.plunkett
#21 routing-names-1981368-21.patch217.45 KBtim.plunkett
#21 interdiff.txt2.17 KBtim.plunkett
#19 route-1981368-19.patch215.66 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +DX (Developer Experience), +WSCCI-conversion

Couple of more tags...

Crell’s picture

Crell CreditAttribution: Crell commented

I think the really early conversions were using dots, and then somewhere along the line we switched to underscores for, um, I don't recall why.

IMO, this is an obvious bikeshed issue for which we should simply follow existing Symfony convention in order to avoid the bikeshed discussion. I don't know off the top of my head what the Symfony convention is, but it's easy enough for us to find out.

Dustin@PI’s picture

Dustin@PI CreditAttribution: Dustin@PI commented
Assigned: Unassigned » Dustin@PI
Status: Active » Reviewed & tested by the community

In light of: https://groups.drupal.org/node/315498, lets get this one fixed.

It appears that this is moving to "module_routename":

(1) New routes all seem to be using underscore: http://drupalcode.org/project/drupal.git?a=search&h=refs%2Fheads%2F8.x&s...

(2) Which matches the examples from symphony2: http://symfony.com/doc/current/book/routing.html & http://symfony.com/doc/current/components/routing/introduction.html

I'll create the issues and patches for the other the remaining modules.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: [policy, then patch] Settle on a naming convention for routes » Convert all routes to 'module_foo' naming convention
Status: Reviewed & tested by the community » Active
Issue tags: +Approved API change

Awesome, thanks for looking into that!

Let's do it all in one patch, here in this issue. The "100000 individual issues that each do one small part of an overall thing" approach adds significant overhead to patch creators, reviewers, and committers.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Can we please do module.foo_bar?

Meaning, we namespace it, and then have underscores for any spaces after that.

Here were some of the very first routes done, and I wish we had kept to that standard, and now we have a chance!

views_ui.settings.basic:
  pattern: '/admin/structure/views/settings'
  defaults:
    _form: '\Drupal\views_ui\Form\BasicSettingsForm'
  requirements:
    _permission: 'administer views'

views_ui.settings.advanced:
  pattern: '/admin/structure/views/settings/advanced'
  defaults:
    _form: '\Drupal\views_ui\Form\AdvancedSettingsForm'
  requirements:
    _permission: 'administer views'
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Convert all routes to 'module_foo' naming convention » Convert all routes to 'namespace.foo' naming convention

Actually, hold the phone.

Mark and Tim brought up that the reason Views does it the way it does it is it's following a "namespace.route_name" pattern. While the Symfony docs don't really do this, that's because they're not dealing with a concept like Modules. Symfony CMF, on the other hand, does. Here's a code sample from http://symfony.com/doc/master/cmf/book/routing.html illustrating this:

services:
    my_namespace.my_router:
        class: "%my_namespace.my_router_class%"
        tags:
            - { name: router, priority: 300 }

Given there's precedent for this in the Symfony world, I actually think we should switch to this convention, since it's going to be a much better guarantee of unique route names across core/contrib.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Convert all routes to 'namespace.foo' naming convention » Convert all routes to 'module_name.foo_bar' naming convention

Slight clarification.

Crell’s picture

Crell CreditAttribution: Crell commented

I'm +1 on modulename.some_key. It's very easy to tell module it comes from, and it avoids most name collision issues.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: -DX (Developer Experience), -Coding standards, -WSCCI, -WSCCI-conversion

Just a note that the examples in #5, such as "views_ui.settings.basic", do not follow the convention that is being proposed here, right? It should be views_ui.settings_basic ?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Views UI was the first module to use routes, so I had no convention to follow.

I chose module.optionalNamespace.someDescription, only using underscores for the module name

views_ui.list
views_ui.add
views_ui.settings.basic
views_ui.settings.advanced
views_ui.reports.fields
views_ui.reports.plugins
views_ui.operation
views_ui.clone
views_ui.delete
views_ui.autocomplete
views_ui.edit
views_ui.edit.display
views_ui.preview
views_ui.breakLock
views_ui.form.addItem
views_ui.form.editDetails
views_ui.form.reorderDisplays
views_ui.form.analyze
views_ui.form.rearrange
views_ui.form.rearrangeFilter
views_ui.form.display
views_ui.form.configItem
views_ui.form.configItemExtra
views_ui.form.configItemGroup

The views_ui.edit, views_ui.settings, and views_ui.form prefixes all have multiple routes, and I used that as a subnamespace beyond the module name.

But by what we discussed on IRC, that would lead to

- views_ui.form.configItemGroup
+ views_ui.form_config_item_group

And that seems less good.

--------

It would be good to have consensus here by tomorrow, so that we can get a patch for this done right as we're switching gears on the route conversions.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

I maintain we need to do what Symfony/Symfony CMF does here so people familiar with those things see familiar stuff in Drupal. Symfony does not use camelCase. It also does not mention multiple namesapces. So module.underscore_separated_thing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: +DX (Developer Experience), +Coding standards, +WSCCI, +WSCCI-conversion

Sorry about the tags. :(

I just wanted to make sure we were agreeing to a standard of

module_name.whatever_you_want_to_put_with_underscores_only

I think that is what is being proposed... I'll make an issue summary. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Title: Convert all routes to 'module_name.foo_bar' naming convention » [policy, then patch] Convert all routes to 'module_name.foo_bar' naming convention
Status: Active » Needs review

Summary updated. All in favor of proposal in summary? Opposed?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Related: #2055857: [policy, then patch] Standardize service names

Crell’s picture

Crell CreditAttribution: Crell commented

#12 is fine by me. Views is (as usual) an edge case but I think views_ui.form_config_item_group is an acceptable route name. Consistency with Symfony CMF makes sense, as it's probably the most similar project in the Symfony-verse.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: Dustin@PI » tim.plunkett

Working on this. Sorry @Dustin@PI.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Please wait for this until the simple wscci conversions start, as there it is not worth to break existing patches for that.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Actually, I'd prefer to do it before they start, so we don't have yet more crap to go back and clean up. Tim and I are planning to work on this on Sunday.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
route-1981368-19.patch215.66 KB

I'll still have to do another pass tomorrow after all the remaining conversions go in, but I wanted to post this as a start.

Locally I have this split up as a commit per module, and then commits for the test modules. Just in case we want to break it up.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
interdiff.txt2.17 KB
routing-names-1981368-21.patch217.45 KB

Always good to get the easy stuff out of the way :)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
interdiff.txt7.74 KB
route-names-1981368-22.patch225.39 KB

Rerolled after the last set of conversions.

+++ b/core/modules/field_ui/field_ui.module
@@ -148,7 +148,7 @@ function field_ui_menu() {
-          'route_name' => "field_ui.display_overview.$entity_type.$view_mode",
+          'route_name' => "field_ui.display_overview_$entity_type" . '_' . $view_mode,

+++ b/core/modules/views_ui/views_ui.routing.yml
@@ -207,7 +207,7 @@ views_ui.form.configItem:
-views_ui.form.configItemExtra:
+views_ui.form_config_item_extra:

Let the record show that I think this is a shame :)

However, having everything prefixed properly is a definite gain here.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Yeah, we could always discuss re-introducing "sub-namespaces" later, and come up with some guidelines on when we do/do not do that. But this is a fairly non-controversial first step towards consistency and sanity.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
interdiff.txt6.24 KB
route-names-1981368-23.patch231.63 KB

Missed a spot!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/lib/Drupal/Core/Entity/EntityManager.php
@@ -434,7 +434,7 @@ public function getAdminRouteInfo($entity_type, $bundle) {
-      'route_name' => 'field_ui.overview.' . $entity_type,
+      'route_name' => "field_ui.overview_$entity_type",

+++ b/core/modules/block/custom_block/custom_block.module
@@ -46,7 +46,7 @@ function custom_block_menu_local_tasks(&$data, $router_item, $root_path) {
-    return "block_admin_display.$theme";
+    return "block.admin_display_$theme";

Here are some examples as a counter-point of our current seemingly random usages of dots. :) There's also some camelCase in the old code sporadically, too.

+++ b/core/modules/action/action.module
@@ -61,17 +61,17 @@ function action_menu() {
   $items['admin/config/system/actions/add'] = array(
     'title' => 'Create an advanced action',
     'type' => MENU_VISIBLE_IN_BREADCRUMB,
-    'route_name' => 'action_admin_add',
+    'route_name' => 'action.admin_add',

+++ b/core/modules/taxonomy/taxonomy.routing.yml
@@ -1,74 +1,74 @@
+taxonomy.vocabulary_add:
   pattern: '/admin/structure/taxonomy/add'

There are still a few outstanding consistency items like this. However, I feel like we maybe don't need to get that anal about whether it's "module.admin_add" everywhere or whether it's "module.thing_add" and just leave it up to module developers to do whatever makes sense for them.

So I've read through this entire patch and it does what it says on the tin. Fixes some indentation issues in a YAML file or two as it goes along. This patch makes my brain go "Ahhhhh" and sip a cool glass of ice tea on a beach.

Marking RTBC, and I'll commit it shortly so we have a baseline against which to do the remaining WSCCI conversions.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: [policy, then patch] Convert all routes to 'module_name.foo_bar' naming convention » [Change notice] Convert all routes to 'module_name.foo_bar' naming convention
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

It's green! It's green! OMG! It's green!

Committed and pushed to 8.x. YEAH.

Needs a change notice. I'll also alert the meta conversion issue.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: [Change notice] Convert all routes to 'module_name.foo_bar' naming convention » Convert all routes to 'module_name.foo_bar' naming convention
Status: Active » Fixed
Issue tags: -Needs change record

Added a change notice here: https://drupal.org/node/2089605

Also updated https://drupal.org/node/1800686 and and https://drupal.org/node/1953346 and https://drupal.org/node/2004268. https://drupal.org/node/1851490 was already using that convention. Couldn't find any others.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Fixed » Needs review
FileSize
route-names-1981368-28.patch1.66 KB

Really sorry about this, but I actually changed a couple form IDs (they matched the route name and I was sloppy).

This would ruin someone's day if they tried to hook_form_alter those

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Perfect

Status: Reviewed & tested by the community » Needs work

The last submitted patch, route-names-1981368-28.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Fixed

Committed and pushed that to 8.x.

YesCT’s picture

YesCT CreditAttribution: YesCT commented

@tim.plunkett says we will want to use the same dot separator modulename . whatever for the local_tasks.yml

I updated the change record there. https://drupal.org/node/2044515/revisions/view/2847981/2850471

And opened #2095613: Convert all plugin IDs in local_tasks.yml to 'module_name.foo_bar' naming convention to match routing convention

YesCT’s picture

YesCT CreditAttribution: YesCT commented

maybe some were missed or are new. examples in #2095271-30: Add default tabs for routes expected by config_translation

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue summary: View changes

Add issue summary

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Assigned: tim.plunkett » Unassigned

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

noting which patches committed.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

People who worked on this may also be interested in #2178725: Make all core menu link machine names and the corresponding route names match, where it's being discussed whether to stick with what was committed here for routes, or change the pattern.