Problem/Motivation

Small issue to convert use of \Drupal\Core\Routing\Enhancer\RouteEnhancerInterface from \Drupal\Core\Routing\EnhancerInterface

This isn't a hard blocker for module just a problem for pass tests in 8.5 branch thats would be great to merge it onto dev branch fast to unblock the situation for image_widget_crop / entity_browser.

Proposed resolution

Convert all RouteEnhancerInterface usage with new one and implement a new method to evaluate of condition are satisfated to arround complexity problem of methods (extract method).

@see https://www.drupal.org/node/2894934

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 convert_deprecated-2922297-15.patch2.14 KBberdir
#4 convert_deprecated-2922297-4.patch1.83 KBwoprrr
#2 convert_deprecated-2922297-2.patch0 byteswoprrr

Comments

woprrr created an issue. See original summary.

woprrr’s picture

woprrr commented
Status: Active » Needs review
StatusFileSize
new convert_deprecated-2922297-2.patch0 bytes

Very small change :D

Status: Needs review » Needs work

The last submitted patch, 2: convert_deprecated-2922297-2.patch, failed testing. View results

woprrr’s picture

woprrr commented
Status: Needs work » Needs review
StatusFileSize
new convert_deprecated-2922297-4.patch1.83 KB

Small miss during patch exporting :O sorry for noise.

woprrr’s picture

woprrr commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 4: convert_deprecated-2922297-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

geertvd’s picture

geertvd commented

The fails in Drupal\Tests\ctools_entity_mask\Functional\DisplayTest and Drupal\Tests\ctools_entity_mask\Kernel\EntityMaskTest aren't related to this patch (they were already failing https://www.drupal.org/pift-ci-job/807372)

The fail in Ctools.Drupal\ctools\Tests\Wizard\CToolsWizardTest does seem like a problem

geertvd’s picture

geertvd commented
Status: Needs work » Reviewed & tested by the community

Actually the fail Ctools.Drupal\ctools\Tests\Wizard\CToolsWizardTest was already occurring before also so this looks good.

woprrr’s picture

woprrr commented

Thank for fast review @geertvd :)

Yes, tests fails before patch, Now we have fixed all noise generated by deprecated enhancer. I create a follow-up issue to solve Tests #2922552: Resolve failed tests

woprrr’s picture

woprrr commented

All green with this issue AND #2922552: Resolve failed tests

:) This have a chance to fast merge on dev branch that should be GREAT :)

woprrr’s picture

woprrr commented
Related issues: +#2922257: Convert ImageWidgetCropExamples Functional WebTestBase to BrowserTestBase

I just attached ImageWidgetCrop issue waiting this merged in dev branch :)

lpsolit’s picture

lpsolit commented

Committers: ping?

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Reviewed & tested by the community » Needs review

This may need a minimum core version as these interfaces don't exist before 8.5. I looked in 8.4 and the interface EnhancerInterface doesn't exist.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

To be honest, everyone who is still on 8.3 or older deservers to get a fatal error, not sure we need to bother with that.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: -convert
StatusFileSize
new convert_deprecated-2922297-15.patch2.14 KB

Add a dependency on 8.5, just to be sure :)

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Thanks for changing your mind on the version bump:)

  • joelpittet committed f3d91d3 on 8.x-3.x authored by Berdir 
    Issue #2922297 by woprrr, Berdir, geertvd, joelpittet, LpSolit: Convert...
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

The 8.7 test failures will be looked at in another issue, they are unrelated. Thanks for all your help on this!

Status: Fixed » Closed (fixed)

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