#2642786: Improve dependency injection in BlockDisplayVariant in CTools changed the dependencies injected to BlockDisplayVariant.

We need to update PanelsDisplayVariant for those changes!

Comments

dsnopek created an issue. See original summary.

dsnopek’s picture

Status: Active » Needs work
StatusFileSize
new2.66 KB

This should be the functional changes! I'm going to try the tests now and see if they need any updates.

We shouldn't commit this until after the next CTools release (which means committing just before we release Panels).

dsnopek’s picture

StatusFileSize
new4.96 KB

Alright, I messed up the order in the last patch. But the tests caught it too! Here's an updated patch which also has all the tests passing for me locally.

dsnopek’s picture

StatusFileSize
new4.96 KB

The last patch didn't include the fix for the order of the arguments. This is really the right patch! :-)

dsnopek’s picture

Just a note: We'll need to update this patch once #2636478: IPE shouldn't depend on Page Manager is merged, because that patch includes a hack around the problems in CTools that #2642786: Improve dependency injection in BlockDisplayVariant fixed. The patch here should also remove that hack.

dsnopek’s picture

StatusFileSize
new3.26 KB

Here's an attempt to update this after #2636478: IPE shouldn't depend on Page Manager - it passes tests locally!

dsnopek’s picture

Status: Needs work » Needs review

CTools has been released - let's give this to the testbot!

Status: Needs review » Needs work

The last submitted patch, 6: panels-bdv-ctools-2644104-6.patch, failed testing.

The last submitted patch, 6: panels-bdv-ctools-2644104-6.patch, failed testing.

The last submitted patch, 6: panels-bdv-ctools-2644104-6.patch, failed testing.

dsnopek’s picture

I have no idea why this is failing. :-/ It just says fatal error, but doesn't give the error, and locally the test passes fine.

dsnopek’s picture

Status: Needs work » Needs review
StatusFileSize
new3.26 KB

Let's try re-uploading this, just in case that helps?

Status: Needs review » Needs work

The last submitted patch, 12: panels-bdv-ctools-2644104-12.patch, failed testing.

dsnopek’s picture

Ooooh, crap! I think I understand why this is failing. PanelsTest depends on using Page Manager do a functional test of Panels, and Page Manager is broken until #2642840: Update page_manager for changes in CTools 3.0-alpha21 is committed. :-/ So, we need to commit that and do a Page Manager release before the Panels tests will pass...

dsnopek’s picture

Status: Needs work » Needs review

Page Manager has finally had it's release:

https://www.drupal.org/node/2647480

Trying the tests again. Come on testbot!

Status: Needs review » Needs work

The last submitted patch, 12: panels-bdv-ctools-2644104-12.patch, failed testing.

The last submitted patch, 12: panels-bdv-ctools-2644104-12.patch, failed testing.

The last submitted patch, 12: panels-bdv-ctools-2644104-12.patch, failed testing.

eclipsegc’s picture

Status: Needs work » Fixed

Ok, we sorted the page_manager dependency difficulties and this is passing now. Fixed

Eclipse

  • EclipseGc committed 6f7cf16 on 8.x-3.x authored by dsnopek
    Issue #2644104 by dsnopek: Update for BlockDisplayVariant changes in...

Status: Fixed » Closed (fixed)

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