Problem/Motivation

ContextAwarePluginBase provides the method setContext() as its primary setter.
It also provides setContextValue() as a convenience method.

However, setContextValue() currently bypasses setContext(), so any class that wants to override one setter must override the other.

Proposed resolution

Make setContextValue() use setContext() internally

Remaining tasks

N/A

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett created an issue. See original summary.

tim.plunkett’s picture

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

This looks good and is both a nice clean-up, and a nice bug fix. A perfect example of why I always advocate for classes to call their own methods as often as possible.

RTBC when the fail patch is red and the pass patch is green.

The last submitted patch, 2: 3014851-setcontextvalue-2-FAIL.patch, failed testing. View results

alexpott’s picture

As an aside it's interesting that setContextValue() returns $this and setContext() does not.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 7dcb927309 to 8.7.x and 136dd9ed0b to 8.6.x. Thanks!

  • alexpott committed 7dcb927 on 8.7.x
    Issue #3014851 by tim.plunkett: ContextAwarePluginBase::setContextValue...

  • alexpott committed 136dd9e on 8.6.x
    Issue #3014851 by tim.plunkett: ContextAwarePluginBase::setContextValue...

Status: Fixed » Closed (fixed)

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

tim.plunkett’s picture

Version: 8.7.x-dev » 8.6.x-dev