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.
Updated: Comment #0
Problem/Motivation
Create BreadcrumbBuilderBase and remove the boilerplate code from BreadcrumbBuilders
Proposed resolution
Create BreadcrumbBuilderBase and remove the boilerplate code from BreadcrumbBuilders
Remaining tasks
Review/RTBC the patch
User interface changes
None
API changes
Clean up
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#9 | breadcrumb-2099095-9.patch | 17.85 KB | tim.plunkett |
#6 | drupal8.base-system.2099095-6.patch | 17.87 KB | jibran |
#6 | interdiff.txt | 3.37 KB | jibran |
#3 | drupal8.base-system.2099095-3.patch | 17.97 KB | jibran |
BreadcrumbBuilderBase.patch | 17.66 KB | jibran | |
Comments
Comment #1
larowlanJust like that.
I can't fault it.
Can this be added to the DX initiative meta (if not already there).
Comment #2
jibranWorking on the reroll
Comment #3
jibranReroll after #731724: Convert comment settings into a field to make them work with CMI and non-node entities.
Comment #5
Crell CreditAttribution: Crell commentedSince BreadcrumbBuilderBase already implements BreadcrumbBuilderInterface, that's not needed on any of the subclasses.
Otherwise, this looks consistent with what we discussed in Prague so +1. (Although I'd still prefer to include the setters for test overriding on the base classes to make them easier to use; we can very easily document that they're for testing purposes only.)
Comment #6
jibran@Crell Nice catch. Fixed #5
Comment #7
Crell CreditAttribution: Crell commentedForward!
Comment #8
alexpottPatch no longer applies.
Comment #9
tim.plunkettSomeone else edited comment.services.yml. No change.
Comment #10
andypostBack to RTBC
Comment #11
catchLooks great. Committed/pushed to 8.x, thanks!
Could use a change notice for the API addition.
Comment #12
jibranUpdated https://drupal.org/node/2026025/revisions/view/2855137/2868077. Added https://drupal.org/node/2106757.
PS: I have created the issue and marked it as fixed. Thanks everybody :).
Comment #13
jibran