Problem/Motivation

When we deprecated Place blocks, we added a trigger_error() to the module file itself, but this makes it complicated to handle the deprecation in tests.

see #3084472: Remove Place Blocks module and #3062302: Properly deprecate the entity reference module.

Proposed resolution

Remove the trigger_error() from the .module file, leaving the rest.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3116399.patch590 bytescatch

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3116399.patch590 bytes

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Version: 9.1.x-dev » 8.9.x-dev
Status: Needs review » Reviewed & tested by the community

Thanks for the patch.

1) Changes seem fine.

2) Patch applies cleanly to 8.9 but is not relevant to D9 as the module was removed.

[mac:kristen:drupal-8.9.x-dev]$ patch -p1 < 3116399.patch 
patching file core/modules/block_place/block_place.module

3) Searching for deprecation in block_place and they are still in:

block_place.libraries.yml
block_place.services.yml
block_place.module (in comments and help)
block_place.install
block_place.info.yml
DisplayVariant/PlaceBlockPageVariant.php

which is very thorough.

4) Tests pass.

5) Moving back to 8.9 and marking RTBC though I'm not sure about manual testing.

xjm’s picture

xjm
she/her
Primary language English
commented

I don't have a terribly strong opinion about this, but is un-deprecating the module file this late actually worthwhile?

  • xjm committed cff4972 on 8.9.x 
    Issue #3116399 by catch, Kristen Pol: Place blocks .module file should...

  • xjm committed 793089d on 8.8.x 
    Issue #3116399 by catch, Kristen Pol: Place blocks .module file should...
xjm’s picture

xjm
she/her
Primary language English
commented
Title: Place blocks .module file should not trigger a deprecation » [D8 only] Place blocks .module file should not trigger a deprecation
Version: 8.9.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Fixed

Discussed with @catch whether this was still worth backporting -- per @catch, we worked around the problem in core by changing tests.

The fact that it was disruptive for core though means contrib/custom tests might have the same problem, and since this is the only model of a whole-module deprecation so far, maybe it's worth also making that example less disruptive. I filed #3135100: [policy and meta] Provide a proper mechanism for deprecating modules and themes to figure this out properly before D10.

Un-deprecating something isn't really disruptive; it just means one less error in Upgrade Status etc. when there are still many errors. I also backported this to 8.8.x because most modules will be trying to bridge compatibility from 8.8 to 9.0, so un-deprecating only for 8.9 doesn't help them really.

Committed to 8.9.x and 8.8.x. Thanks!

Status: Fixed » Closed (fixed)

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