Problem/Motivation

#2760167-185: Add \Drupal\Core\Messenger\Messenger asked that this follow-up issue be created.

Proposed resolution

Investigate an alternative way to storing LegacyMessnger messages, perhaps as a static property on itself?

Remaining tasks

Create patch

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#12 2931264-11.patch5.82 KBmarkhalliwell
#6 2931264-6.intediff.txt460 bytesclaudiu.cristea
#6 2931264-6.patch5.21 KBclaudiu.cristea
#2 2931264-2.patch5.6 KBmarkhalliwell

Comments

markcarver created an issue. See original summary.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Active » Needs review
Parent issue: #2760167: Add \Drupal\Core\Messenger\Messenger »
StatusFileSize
new 2931264-2.patch5.6 KB

This may work after all. Tested locally, this should pass.

Status: Needs review » Needs work

The last submitted patch, 2: 2931264-2.patch, failed testing. View results

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs work » Needs review

apcu memory failure, retesting

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

This looks much better.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
StatusFileSize
new 2931264-6.patch5.21 KB
new 2931264-6.intediff.txt460 bytes 
+++ b/core/lib/Drupal/Core/Messenger/LegacyMessenger.php
+++ b/core/lib/Drupal/Core/Messenger/LegacyMessenger.php
@@ -26,9 +26,13 @@ class LegacyMessenger implements MessengerInterface {

@@ -26,9 +26,13 @@ class LegacyMessenger implements MessengerInterface {
-  protected $messages;
+  static protected $messages;

Nit: Even in our coding standards is not yet stated, in core we use to start with the visibility ('protected'), followed by 'static'. There's an issue for this #2855980: Define order of object method modifiers as in PSR-12 that most likely will be adopted but it's also a PSR-2 recommendation.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2931264-6.patch, failed testing. View results

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Reviewed & tested by the community

apcu memory failure

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2931264-6.patch, failed testing. View results

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

This apcu stuff is getting really annoying

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2931264-6.patch, failed testing. View results

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 2931264-11.patch5.82 KB

Reroll

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs review » Reviewed & tested by the community

This was RTBC before reroll.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed 984a268 and pushed to 8.5.x

Thanks

  • larowlan committed 984a268 on 8.5.x 
    Issue #2931264 by markcarver, claudiu.cristea: Remove static \Drupal::$...

Status: Fixed » Closed (fixed)

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