Problem/Motivation

These two events, PreDestroyEvent and PostDestroyEvent, do not have a very good value-to-complexity ratio. We barely use them -- the one legitimate place where we use PostDestroyEvent could be non-disruptively changed to PostApplyEvent -- and they are complicated to support, since the stage is "unlocked" during destroy, which makes testing tricky, particularly with PostDestroyEvent. They also make our API surface bigger.

Steps to reproduce

N/A

Proposed resolution

Remove these events entirely.

Issue fork automatic_updates-3383451

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

phenaproxima created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes

phenaproxima opened merge request !949

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review

Welp, that was pretty easy.

Before we merge this, we should definitely discuss (in scrum?) to see if we truly cannot think of a use for the destroy events. I'd be comfortable removing them, as long as we're certain we can't imagine any remotely mainstream case where they'd be necessary.

tedbow made their first commit to this issue’s fork.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: Unassigned » phenaproxima

@phenaproxima I pushed up a few commits to clean this up.

I think we have since talked about this in scrum.

If you are good with my changes feel free to RTBC and merge.

I think we definitely want this out before we try to get core reviews and definitely before core commit. I think it will super hard to remove them after the fact.

phenaproxima’s picture

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

This looks great to me. Really nice to remove the complexity introduced by these events.

I did not find any more instances of DestroyEvent or destroy event in the code base. So this is gonna happen.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

  • phenaproxima committed 152eb59c on 3.0.x 
    Issue #3383451 by phenaproxima, tedbow: Remove PreDestroyEvent and...
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: phenaproxima » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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