#DX (Developer Experience)

Ok, so this needs backporing to 11.0.x.

At the moment this trait has an architectural flaw - the logging expectations do not survive a container rebuild which they need to.

We can convert \Drupal\KernelTests\Core\Extension\ModuleInstallerTest, \Drupal\Tests\system\Kernel\SecurityAdvisories\SecurityAdvisoriesFetcherTest and \Drupal\KernelTests\Core\Config\ConfigImporterMissingContentTest are converted to use this.

Great job!

Splitting the header into separate lines with the same header name is going to be difficult, given that the Symfony Response object's headers object doesn't allow for multiple headers with the same key.

Also, if the goal here is only to fix headers for the debug cache tags, having the -1, -2 suffixes probably works just fine. If this issue also needs to address headers generally that are too long (such as the ones used by Purge), the current MR does not address that, and the question of whether and how to split headers into multiple lines with the same name is relevant.

I am worried about how disruptive this change could be. Since both @catch and @daffie have commented on this issue, and neither seems concerned about that, I guess it is OK. But I am adding the tag for a change record (NW for that) and I am adding a release-notes snippet to the issue summary.

As @erikbj also mentions in #131, the official proper way to handle headers with too many values is indeed to split them up into separate lines but with the same header name (see https://www.rfc-editor.org/rfc/rfc9110.html#section-5.2). This would also ensure it remains easy to implement downstream logic basic on the header ("if header contains xyz" is a lot easier than "if header-1 contains xyz or header-2 contains xyz or header-3 contains xyz ...")

So I think splitting the headers but keeping the name is preferable over the current solution appending '-{delta}'. One caveat however: the different header line values are combined with ",", which means the current implementation would have to be converted to generate a comma-separated list instead of a space-separated list (which in turn is not backwards-compatible). On the other hand, this is meant to be a debugging tool, so I don't think this is that big a deal.

> Already in core — see this for ckeditor5.plugin.ckeditor5_language for example:

Yes but that's only the option values. It doesn't define the option labels.

The labels are needed for decoupled UIs, and also for translation.

Patch #161 not working with 10.4.x, which is supposed to be LTS for D10. Tested patch created from the merge request's latest commit ID 021da95f and works with 10.4.x, adding patch here.

Here's patch at #314 re-rolled for D7.103

@gresko8
Your re-rolled patch lacks my changes at #314.

You seem to have replaced $kernel wit $http_kernel everywhere, not just in the classes concerned in this issue, and also lots of places where it's actually the DrupalKernel!

This issue is only about middleware classes.

Also, please can you make a MR rather than a patch?

I have created the patch please review it

Going through (really) old issues to clean up the queue.

While I agree renderLink is indeed not great, buildLink still doesn't tell me what this method does, so to me that isn't worth changing this over. To me, the problem is that this method does two things. It updates settings to make sure we are going to be rendering a link and then returns a string to use in the link. So, to me, we should either split this up into two methods or make sure the name represents what it does: setRenderAsLinkAndGetLinkText() which feels very verbose but at least tells you what it does ¯\_(ツ)_/¯

dubois → changed the visibility of the branch 2844620-finishresponsesubscriber-need-warningerror to active.

$ npm run drupaldev
> vite-template-redux@0.0.0 drupaldev
> VITE_DRUPAL=true vite
  VITE v5.4.11  ready in 289 ms
  ➜  Local:   http://localhost:5173/
  ➜  Network: use --host to expose

… oops, npm run drupaldev is long-running 😅

I'll let @balintbrews adjust that! 🤓

wim leers → credited balintbrews → .

Settings.php overrides are unsuitable for the modules that try to control the backend of the queue worker.
I like the idea of keeping the current approach to override the backend service.

What if, instead of extending attribute/annotation, we introduce an extension point by dispatching an event right after determining the service name?

The definition is the annotation and the MR updates the annotation.

This would need to update the attribute class now as well.

I'm not really convinced this is a good idea. I see that there are sometimes use cases where you implement a custom queue plugin and want to use a specific storage at the same time and that currently requires an extra line in settings.php, but I'd argue it's at least as common to use a certain backend for a queue plugin that you didn't implement yourself and this now requires a module with custom code, that might need its own set of environment-specific control to only do that on certain environments. Maybe we support both without deprecating the existing approach, not really a reason to not support both? The cache backend is similar in that cache bins can have a default backend that can be overridden.