Problem/Motivation

Add object that uses the FactoryInterface should have dependencies inject via ::create and not ::__construct(). This allows parent classes to change their constructor without breaking any code.
@see https://events.drupal.org/seattle2019/sessions/drupal-9-coming-getting-y...

Example: \Drupal\config_sync\Controller\ConfigSyncController::create
@see https://www.youtube.com/watch?time_continue=1554&v=hN9KjaBvAUk
@see https://www.previousnext.com.au/blog/safely-extending-drupal-8-plugin-cl...

Proposed resolution

Fix subclassing and stop overriding constructors.

CommentFileSizeAuthor
#10 3112913-subclassing-SlickManager-10.patch5.63 KBgausarts
#2 slick-Fix_subclassing_and_stop_overriding_constructors_in_slick_SlickManager-3112913-2-D8.patch1.39 KBthalles

Comments

thalles created an issue. See original summary.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Assigned: Unassigned » gausarts
Status: Needs work » Needs review
StatusFileSize
new slick-Fix_subclassing_and_stop_overriding_constructors_in_slick_SlickManager-3112913-2-D8.patch1.39 KB

Follow a patch, but some adjustments are still missing

Status: Needs review » Needs work

The last submitted patch, 2: slick-Fix_subclassing_and_stop_overriding_constructors_in_slick_SlickManager-3112913-2-D8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gausarts’s picture

gausarts commented
Assigned: gausarts » thalles

Minor adjustments to test files are needed, I guess, and this should be green. Let me know if you can follow up. Thank you!

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Hi @gausarts, I didn't find these errors, where I need to change, can you do that?

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented
Assigned: thalles » gausarts
gausarts’s picture

gausarts commented
Assigned: gausarts » Unassigned
Status: Needs work » Closed (won't fix)

I guess this part is no go without extra legs for now. Only plugin system takes this advantage immediately. Feel free to re-open if you are working on it, and or continue working with plugin system instead. Thanks!

gausarts’s picture

gausarts commented
Status: Closed (won't fix) » Postponed

Postponed this lovely idea till we have time and figure out things better for the extra legs.

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Ok!

gausarts’s picture

gausarts commented
Status: Postponed » Needs review
StatusFileSize
new 3112913-subclassing-SlickManager-10.patch5.63 KB

Let's give way for Blazy improvements with DI, and see if any issue.
Also added minor fixes from the previous SlickTextFormatter patch not caught by tests, nor me.

  • gausarts committed 348114e on 8.x-2.x 
    Issue #3112913 by thalles, gausarts: Fix subclassing and stop overriding...
gausarts’s picture

gausarts commented
Status: Needs review » Fixed

Let's get the ball rolling, we have another release to catch up. Any (side) issue and or improvements may follow as always.

Committed. Thank you for contribution!

thalles’s picture

thalles
Primary language Portuguese, Brazil
Location Teófilo Otoni - MG
commented

Thanks @gausarts!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.