- Issue created by @catch
- Status changed to Needs review
4 months ago 1:03pm 21 July 2024 - π¬π§United Kingdom catch
The test coverage removed here was added in #2378263: hook_library_alter() must be manually invoked by users of LibraryDiscovery, and has no test coverage β , this add a hook_library_alter:
public function getLibrariesByExtension($extension) { - return $this->collector->get($extension); + if (!isset($this->libraryDefinitions[$extension])) { + $libraries = $this->collector->get($extension); + $this->libraryDefinitions[$extension] = []; + foreach ($libraries as $name => $definition) { + // Allow modules and themes to dynamically attach request and context + // specific data for this library; e.g., localization. + $library_name = "$extension/$name"; + $this->moduleHandler->alter('library', $definition, $library_name); + $this->libraryDefinitions[$extension][$name] = $definition; + }
Then we removed hook_library_alter() in #2472547: Remove deprecated hook_library_alter() β and the test was left testing the static caching in LibraryDiscovery, but without checking whether it was still necessary or not.
- π¬π§United Kingdom catch
Let's do the deprecation here.
Once it's merged, a couple of the methods are redundant, so we should probably deprecate those too, but maybe those in a follow-up.
- Status changed to Needs work
4 months ago 12:27am 22 July 2024 The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- Status changed to RTBC
4 months ago 1:57am 22 July 2024 - Status changed to Needs review
4 months ago 2:16am 22 July 2024 - Status changed to RTBC
4 months ago 2:45pm 22 July 2024 - πΊπΈUnited States smustgrave
Deprecation seems fine, version wise.
Not 100% if all deprecations need a test to make sure the deprecation is returned. I feel like we know that feature works.
- π¬π§United Kingdom catch
Not 100% if all deprecations need a test to make sure the deprecation is returned. I feel like we know that feature works.
Generally I think we should not do this, and only test deprecations triggered by actual backwards compatibility logic rather than from within deprecated code that's going to be removed. It takes time to write the test, run them every CI run, then remove them again, and they don't tell us much.
- Status changed to Needs work
4 months ago 4:40pm 26 July 2024 The Needs Review Queue Bot β tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- Status changed to RTBC
4 months ago 1:14am 28 July 2024 - Status changed to Fixed
3 months ago 9:47am 14 August 2024 - π«π·France nod_ Lille
had to fix the commit since π Single directory component CSS asset library not picked up in admin theme immediately after module install without cache clear Active introduced a new use of the deprecated method
Automatically closed - issue fixed for 2 weeks with no activity.