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.
Doing this after #2098795: Create Base Class for RouteSubscriber Class - Done
We are still using module in the RouteBuildEvent class, this doesn't make sense as we are now using 'provider' everywhere else pretty much. Especially as we have 'dynamic_routes' as the current 'module' when altering the dynamic route collection, also, doesn't make sense. Provider this works better.
Comment | File | Size | Author |
---|---|---|---|
#26 | route_buillder-2102417.patch | 12.83 KB | dawehner |
#15 | 2102417-15.patch | 11.68 KB | damiankloip |
#15 | interdiff-2102417-15.txt | 2.68 KB | damiankloip |
#11 | 2102417-11.patch | 9.61 KB | damiankloip |
#8 | 2102417-8.patch | 11.75 KB | damiankloip |
Comments
Comment #1
damiankloip CreditAttribution: damiankloip commentedComment #2
damiankloip CreditAttribution: damiankloip commentedComment #4
damiankloip CreditAttribution: damiankloip commentedDoh, forgot to add the remaining changes to the last patch out.
Comment #5
Crell CreditAttribution: Crell commented+1 on this in concept. Although I think the Dumper uses "route set" or something generic like that. We should probably use the same term there as well. (Either switch to "set" here, or "provider" there. I guess I'm OK with either.)
Comment #6
damiankloip CreditAttribution: damiankloip commentedThat sounds sensible, I think we should go with provider, as this term is being used a fair bit in D8 now.
Comment #8
damiankloip CreditAttribution: damiankloip commentedGah, update schema meh.
Comment #9
dawehnerJust wondering whether it is worth to break other peoples code, if they use the actual RouteSubscriberInterface.
I would be fine with having a little bet less spaces after the ":"
+2!
Do we already provide inter-drupal updates?
Comment #10
Crell CreditAttribution: Crell commentedNo inter-drupal updates at this point.
Comment #11
damiankloip CreditAttribution: damiankloip commentedRemoved the update hook from system.install, was more of a reflex bit of coding :) Also changed that one piece of whitespace. Apart from that, patch is the same.
I think this is definitely worth the change, as making our various APIs more consistent is pretty important I think. If in some cases you are using providers and some modules or route sets, this is much much worse.
Comment #13
damiankloip CreditAttribution: damiankloip commented#11: 2102417-11.patch queued for re-testing.
Comment #15
damiankloip CreditAttribution: damiankloip commentedOops, didn't want to remove all the things from system.install - that was rash.
Comment #16
dawehnerThere is nothing in core anymore which is called 'route_set', so yeah!
Comment #16.0
dawehnerUpdated issue summary.
Comment #17
alexpottComment #18
damiankloip CreditAttribution: damiankloip commentedRerolled
Comment #20
damiankloip CreditAttribution: damiankloip commentedNeed to fix the new RouteBuilder tests..This should be good now.
Comment #21
damiankloip CreditAttribution: damiankloip commentedAnd without the random newline added in the node revision access checker.
Comment #22
dawehnerBack to rtbc
Comment #23
alexpottDoes this mean we can't have a module named dynamic_routes?
Comment #24
Crell CreditAttribution: Crell commentedalexpott: Yes. That's been the case for over a year now. It's unrelated to this issue.
Comment #25
alexpottComment #26
dawehnerRerolled.
Comment #27
Crell CreditAttribution: Crell commentedAnd back.
Comment #28
alexpottCommitted 8e729c3 and pushed to 8.x. Thanks!
Comment #29
dawehnerThank you!