Rename various *links.yml files to improve DX

I don't think that having one file versus four files is really much of a gain. Either it's long and hard to read, or you open four things. To me it makes sense to have them separated conceptually.

Well I guess this might depend on how many menu links, local tasks, local actions, or contextual links a module is providing. If you're providing one of each then four files might feel like overkill. But on the other hand these "links" are not the same thing and they have different data structures. Also the lack of a file makes it easy to discover when a module does not provide something.

Well I guess this might depend on how many menu links, local tasks, local actions, or contextual links a module is providing. If you're providing one of each then four files might feel like overkill.

Just looking through core modules, looks to me like a lot provide 2 or 3.

But on the other hand these "links" are not the same thing

They're both similar and different. They're all links. But they have different UX purposes and therefore get rendered in different places with different hierarchy organization.

and they have different data structures.

Their data structures are both similar and different. They all share the same semantics of title, description, route_name, weight, and class. But related to hierarchy, they differ with respect to parent, parent_id (though that's silly, it should be renamed to parent), base_route, group, and appears_on.

Therefore, in order to communicate/represent the similarities and differences, what about keeping them as separate files, but renaming them to:

• *.links.actions
• *.links.contextual
• *.links.menu
• *.links.tasks

?

I'm hugely -1 on combining these.

However, I'm not averse to renaming them. I never liked "local tasks" and "local actions" anyway, since they're all just links.

Another options is this:

• *.actions.links.yml
• *.contextual.links.yml
• *.menu.links.yml
• *.tasks.links.yml

vim *.links.yml

I'm not against combining them, but between #4 and #5 I'm very much for #4. vi *.links.yml is nice, but so is vi mymodule.links.*. And #4 has the benefit of neatly showing the files next to each other in the the file browser.

The 'subtle' differences are even more spelled out when you want to create dynamic items of hook_menu() successors. Opened this issue to summarize the differences and similarities: #2291773: Creating dynamic hook_menu() successor items use different APIs maybe we want to do something about this, maybe not :)

Having them in separate files certainly helps people to remember that there are differences. Otherwise they really easy might just copy and paste other bits and wonder why they don't work.

*.links.actions
*.links.contextual
*.links.menu
*.links.tasks

I also prefer this variant but it would be great to keep it either singular or plural so:

*.links.action
*.links.contextual
*.links.menu
*.links.task

On top of that having them in one file would make scanning of them a bit worse on the performance side, because as the
subsystems are decoupled all of them would have to read the merged file anyway. Not sure whether this though actually
matters given that it is cached but yeah a full router + menu rebuild is always potentially problematic.

We discussed one file before, I have no idea which issue it was, from what I remember the pros and cons were similar (single file for overview vs. making the separation between systems clear and reducing parsing overhead). Another thing to consider is that (at least) contextual module is optional, so if we have information in one file we're having to parse YAML that will never be used on some sites. There's also the ability to completely ignore the other types altogether if you have a module that just adds a single settings admin page or similar.

So, I'd also like to keep single files, but I like dawehner's second variant in #8.

This is not clear to me: Is the intent that these would actually be named with the YAML file extension?:

*.links.action.yml
*.links.contextual.yml
*.links.menu.yml
*.links.task.yml

I am trying local_tasks here. This will certainly fail the bots.

Updated the local tasks test base. We still needs the other link types.

links.contextual. I am not dealing with mentions in comments yet.

links.menu

Todo: links.action, comment mentions, and change records.

links.action

Todo: comment mentions and change records.

This looks really good so far!

It maybe makes sense to postpone the links.menu_link changes, given that this will conflict with the menu link patches, like #2301239: MenuLinkNG part1 (no UI or conversions): plugins (static + MenuLinkContent) + MenuLinkManager + MenuTreeStorage

Found a few of the comments, but may have missed a few.

Todo: change records. Postpone all or part on #2301239: MenuLinkNG part1 (no UI or conversions): plugins (static + MenuLinkContent) + MenuLinkManager + MenuTreeStorage if necessary, depending on the impact on that issue.

I would be fine with temporarily moving just the three types.

This patch is meant to address only:

*.links.action
*.links.contextual
*.links.task

*.links.menu is being postponed.

Todo:

• Change records
• Open an issue for *.links.menu that is postponed on #2301239: MenuLinkNG part1 (no UI or conversions): plugins (static + MenuLinkContent) + MenuLinkManager + MenuTreeStorage.

The affected change records to be updated and referred-to are:

• https://www.drupal.org/node/2165243
• https://www.drupal.org/node/2007444
• https://www.drupal.org/node/2044515

Draft change notice: https://www.drupal.org/node/2302893

The change notice coverage looks fine as well as the patch.

+1

FWIW, contextual (module) appearing in that list implies that contrib modules are supposed to advance on this pattern by using their own suffix.

At the same time, the suffixes "action", "task", and "menu" do not correlate to a module, but are sorta "reserved" by Drupal core.

I assume that is intended, but worth to point out.

@sun Contrib modules must use the new names. Maybe we could be clearer on that in the change record.

@sun Oh sorry I misunderstood your comment.

Sorry for finding this so late in the discussion. To me, this implies some connection between these system that are pretty disparate. Putting links on the page is just something we do because we write HTML. Ditto on routing.yml, everything is going to point to that, because that's the core of our system.

I guess I just want to make sure we are sure we're not doing this based on the history of hook_menu and our biases as experienced Drupal developers rather then actual clarity.

To me, this implies some connection between these system that are pretty disparate.

If we consider our other module-level YAML files, we have .info.yml, .routing.yml, .services.yml, .libraries.yml, and maybe eventually .events.yml. Those are much more disparate concepts than what variants of link groups are to each other. So I think them staying separate files is a good enough acknowledgement of their disparateness, but putting them all under a common "links" namespace is a good way to group them conceptually.

FWIW, contextual (module) appearing in that list implies that contrib modules are supposed to advance on this pattern by using their own suffix. At the same time, the suffixes "action", "task", and "menu" do not correlate to a module, but are sorta "reserved" by Drupal core.

Well, I think menu is also well named, as that relates to the \Drupal\Core\Menu component. I suppose action and task aren't properly namespaced and are just being grandfathered in: similar problem we have elsewhere in core (e.g., a date_format entity type). Since the lack of component or module namespace for 'local_tasks' and 'local_actions' is a preexisting condition of HEAD, I don't think this patch makes the problem any worse.

Okay let's get this in whilst the patch applies and we've got no new files.

Committed f2b6aa7 and pushed to 8.x. Thanks!

Thanks! This is definitely an improvement to status quo and improves DX.

I would, however, recommend to leave the door open for further improvements. To improve consistency of definitions across files (cf. #4), but also, to discuss and consider a potential merge of e.g. actions and tasks into a single file, in separate issues.

We need to fix documentation as well eg. https://www.drupal.org/search/site/local_tasks.yml?f[0]=ss_meta_type%3Ad...

The https://www.drupal.org/node/2122231 page is a good starting point, since that search does not seem to yield any results.

Hopefully all pages are covered

https://www.drupal.org/node/2302893 mention that also *.menu_links.yml has been renamed to *.links.menu.yml but this seems not true.

@lussoluca You are correct about that.

Followup for links.menu #2303605: Rename *.menu_links.yml to *.links.menu.yml files to improve DX.

Is it proper form to update superseded change records?

https://www.drupal.org/node/2302893 references old change records that may need updating if that is true.

Updated the D8 code in potx translation parser: #2305555: All the links files got renamed in Drupal 8

Is it proper form to update superseded change records?

Yes.

Updated the prior change records. Can someone give them a quick review, update the title of this issue, and mark this "fixed"?

The prior notices are linked from this one:
https://www.drupal.org/node/2302893

These updates looks perfect.