Follow-up from #2068471: Normalize Controller/View-listener behavior with a Page object.

We have a number of "special" controllers now, for various use cases, that are registered as services to make them easily exchangeable. They've evolved in bits and pieces though, so there's some inconsistency in their service names and signatures.

Let's standardize at least the service names, and if appropriate refactor some of the internal code to avoid duplication. (Traits would be good here.)

Marking postponed until the aforementioned issue lands.

Comments

Crell’s picture

Crell commented
Status: Postponed » Active

The aforementioned issue is now in.

xjm’s picture

xjm
she/her
Primary language English
commented

There is no such thing as a normal beta blocker. :) Should this issue be critical, as in, we can't release Drupal 8.0 without it being done? If so, beta blocker makes sense and we should bump the priority. If not, let's use the "beta target" tag instead.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Regardless of the (seemingly more limited?) scope of this issue, I think we should have a critical release-blocking issue for renaming a whole bunch of service IDs:

  1. Proper provider + owner namespacing.

    To primarily make sense in light of (1) the Container::underscore() implementation and (2) Drupal's extensible core/module/implementation/plugin infrastructure.

    The main aspect being usage of dots vs. underscores in service IDs:

    Before:

    cache.backend.database          → Cache_Backend_Database
plugin.manager.field.field_type → Plugin_Manager_Field_Field_Type
plugin.manager.field.formatter  → Plugin_Manager_Field_Formatter

    After:

    cache_backend.database          → CacheBackend_Database
plugin_manager.field.type       → PluginManager_Field_Type
plugin_manager.field.formatter  → PluginManager_Field_Formatter

  2. Consistency.

    Before:

    controller.form
controller.entityform
exception_controller

    After:

    controller.form
controller.form.entity
controller.exception

+ further adjustments along the same lines.

I'm not sure whether that matches @Crell's intended scope of this issue or whether I should create a separate issue for that though?

Crell’s picture

Crell commented
Issue tags: -alpha target, -beta blocker

I don't recall tagging it beta blocker, and I know I've never tagged anything alpha target, so I'm going to scratch my head and blame Dreditor's "make followup" command for those tags. This shouldn't even be an API change except for renaming a handful of services that it's extremely unlikely anyone is going to be overriding any time soon.

As for sun's comment, I am pretty sure we already have a standard for service names: Underscores for word separation, periods for namespacing. (Routes should be using the same.) If that's not being applied consistently that's a task for a separate issue to resolve. This issue is more focused.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +DX (Developer Experience)
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Parent issue: » #2251085: [meta] Drupal 8 pre-beta major disruption window
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

I'd say this is D9 material now, but with the use of BC service aliases, it's still possible to do this.

kgoel’s picture

kgoel commented

@crell - Can you please share your thoughts whether this is still possible to do in D8 or should it mark as postponed?

dawehner’s picture

dawehner
Primary language German
commented
Version: 8.0.x-dev » 8.1.x-dev

Given that we could easily make a BC layer (just service aliases) this could be tackled in 8.1.x.

Crell’s picture

Crell commented

Many of the services we'd be dealing with here are "internal", ie, no module would ever be messing with them in normal circumstances. We could probably still rename them at any time, although tossing an alias into the services.yml file is trivial enough to do.

Crell’s picture

Crell commented
Status: Active » Closed (cannot reproduce)

Actually, this is no longer relevant as only one of the wrapping controllers even still exists in the refactoring that's been done in the last year. So let's just close this out.