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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

woprrr created an issue. See original summary.

woprrr’s picture

woprrr CreditAttribution: woprrr as a volunteer and at NeoLynk commented
Status: Active » Needs review
FileSize
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 CreditAttribution: woprrr as a volunteer and at NeoLynk commented
Status: Needs work » Needs review
FileSize
convert_deprecated-2922297-4.patch1.83 KB

Small miss during patch exporting :O sorry for noise.

woprrr’s picture

woprrr CreditAttribution: woprrr as a volunteer and at NeoLynk 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 CreditAttribution: geertvd at Geert van Dort 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 CreditAttribution: geertvd at Geert van Dort 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 CreditAttribution: woprrr as a volunteer and at NeoLynk 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 CreditAttribution: woprrr as a volunteer and at NeoLynk 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 CreditAttribution: woprrr as a volunteer and at NeoLynk 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 CreditAttribution: LpSolit as a volunteer commented

Committers: ping?

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia 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
CreditAttribution: Berdir 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
CreditAttribution: Berdir commented
Issue tags: -convert
FileSize
convert_deprecated-2922297-15.patch2.14 KB

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

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia 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
Primary language English
Location Vancouver
CreditAttribution: joelpittet at The University of British Columbia 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.