Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Symfony CMF 1.3 got updated
Proposed resolution
Let's update it and drop some code Drupal had and got rewritten more generically in CMF
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#16 | 2363537-16.patch | 195.37 KB | dawehner |
#16 | interdiff.txt | 1.29 KB | dawehner |
#12 | 2363537-12.patch | 195.24 KB | dawehner |
#7 | 2363537-interdiff.txt | 6.31 KB | dawehner |
#7 | 2363537-7.patch | 67.83 KB | dawehner |
Comments
Comment #1
dawehnerComment #2
BerdirI guess you wanted to set this to needs review?
Comment #4
dawehnerSure, ...
Comment #5
Crell CreditAttribution: Crell commentedYummy. :-)
According to PagedRouteProviderInterface, if $length is null then all routes should be returned. That means the range() method should only be called if($length).
(Thanks to znerol for catching this.)
Other than that, I think this looks fine.
Comment #6
catchLibrary update -> critical.
Comment #7
dawehnerHere is an update with some additional tests.
Comment #8
znerol CreditAttribution: znerol commentedOops.
Comment #9
mbrett5062 CreditAttribution: mbrett5062 commentedPlease ignore this if I am being silly, and also if this is not the right time for nitpicks.
Not sure about this, but did you mean to remove "target-dir"?
Nitpick: Trailing whitespace.
Comment #10
aspilicious CreditAttribution: aspilicious commentedComment #11
Crell CreditAttribution: Crell commentedmbrett: composer.lock is a generated file, and any code in /core/vendor is 3rd party code, not ours. Both should be largely ignored.
Comment #12
dawehnerRAGE :) Well just remove it and run
composer install
again ... and you should be done.Having a recent composer version now forces the installed.json file to be written different ... so the patch size increases a lot now.
Comment #13
Crell CreditAttribution: Crell commentedChanges to installed.json accounted for an extra 130 KB of patch? Apparently so. All the more reason that we need to stop checking vendor into Git. :-(
Comment #14
znerol CreditAttribution: znerol commentedThis still does not look 100% correct to me. If something passes 0 or "" (for whatever silly reason), then all routes will be loaded. Why not use
isset($length)
which conforms exactly to the interface.Comment #15
alexpottin IRC
Comment #16
dawehnerThere we go
Comment #17
Crell CreditAttribution: Crell commentedEither way.
Comment #18
webchickDeleting all of that now-unnecessary code is lovely. I was concerned that doing so might break a crap ton of modules during beta, but the hunks in this patch seem to be limited to Composer, Symfony CMF, deleting said unnecessary code, and some additional test coverage. The one exception is core/lib/Drupal/Core/Routing/RouteProvider.php but this seems to be far enough under the covers that the disruption is minimal. Updating this library is also in service of resolving another core issue at #2203431: [meta] Various asset (JavaScript) libraries have to be updated to a (minified) stable release prior to 8.0.0, so I think we are good to go from the standpoint of https://www.drupal.org/contribute/core/beta-changes.
Committed and pushed to 8.0.x. Thanks!