Problem/Motivation

Follow-up to #2278383: Create an injectible service for drupal_set_message()
Leftover @todo in element with ref to closed issue

Proposed resolution

remove the todo

Remaining tasks

review/commit

User interface changes

no

API changes

no

Data model changes

no

CommentFileSizeAuthor
2278383-follow-up-2.patch635 bytesandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Active » Needs review
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Parent issue: » #2278383: Create an injectible service for drupal_set_message()
r.nabiullin’s picture

r.nabiullin CreditAttribution: r.nabiullin as a volunteer and at Skilld commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Needs work

Well someone needs to do the issue forensics to work out if the @todo has indeed been carried out - the @todo was added in #2289917: Convert "messages" page element into blocks and this code was not touched in #2278383: Create an injectible service for drupal_set_message() so how were things to be improved?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Needs work » Reviewed & tested by the community

So the @todo was added as a request in #2289917-127: Convert "messages" page element into blocks - BUT this was a bit strange. This is static code so I have no-idea how this was meant to be improved. Service injection into RenderElement objects is an issue in and of itself - so I guess removing the useless @todo is the thing to do.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Crediting myself for looking at the issue history and working out if removing the @todo was the correct thing to do.

Committed and pushed 92e2f1a3a4 to 8.7.x and 605b5ccfe8 to 8.6.x. Thanks!

  • alexpott committed 92e2f1a on 8.7.x 
    Issue #2985709 by andypost, alexpott: Remove outdated @todo in...

  • alexpott committed 605b5cc on 8.6.x 
    Issue #2985709 by andypost, alexpott: Remove outdated @todo in...

Status: Fixed » Closed (fixed)

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