Problem/Motivation

Right now in our in our validators we have logic like this

if ($stage instanceof ConsoleUpdateStage) {

to allow determining the stage is regular or console. If others do this our stage classes aren't really internal

Steps to reproduce

Proposed resolution

Add getType method to stagebase

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork automatic_updates-3404429

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

tedbow created an issue. See original summary.

tedbow opened merge request !986

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: Unassigned » phenaproxima
Status: Active » Needs review
Issue tags: +core-mvp

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

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

I'm pretty much happy with this, although it occurs to me that we could make type() a protected property or class constant, rather than a method, and still keep our rules about subclasses overridding it (\ReflectionProperty and \ReflectionClassConstant both have the getDeclaringClass() method).

The only downside is that it would no longer have any dynamism, but I'm honestly not sure if we really need that anyway.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

@phenaproxima re #5 I changed this to protected string. I agree it simpler.

The only downside is that it would no longer have any dynamism, but I'm honestly not sure if we really need that anyway.

As long as it is not a constant it can still be somewhat dynanmic.

for instance we could have have our attended and unattended stages be one class with 2 different services using this class. Just add a $more property to the constructor and in the constructor set the $this->type value.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: phenaproxima » tedbow
Status: Needs review » Needs work

Couple of small things but this otherwise looks good to me.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » phenaproxima
Status: Needs work » Needs review

Accepted suggestions and 1 more cleanup

phenaproxima’s picture

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

One suggestion but RTBC overall.

  • tedbow committed b5bb44fb on 3.0.x 
    Issue #3404429: Add getType to StageBase to allow subclasses to be...
phenaproxima’s picture

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

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

ugh the test failure I think is #3397228: Possible random failure in build tests for cron updates. 3.0.x is passing after the merge

Status: Fixed » Closed (fixed)

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