Problem/Motivation

\Drupal\Core\Menu\MenuLinkTree does not document its $menuLinkManager variable.

Proposed resolution

Document it! (Just like all the other variables in the there...)

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#9 after-apply-patch.png60.59 KBamit.drupal
#9 before-apply-patch.png69.15 KBamit.drupal
#8 Drupal Doc Comment.png67.79 KBrenatog
#3 menulinkmanager-2865295-3.patch597 bytesJayKandari
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler created an issue. See original summary.

JayKandari’s picture

JayKandari
Primary language Hindi
CreditAttribution: JayKandari as a volunteer and at QED42 commented
Assigned: Unassigned » JayKandari
JayKandari’s picture

JayKandari
Primary language Hindi
CreditAttribution: JayKandari as a volunteer and at QED42 commented
Assigned: JayKandari » Unassigned
Status: Active » Needs review
FileSize
menulinkmanager-2865295-3.patch597 bytes

Changes applied. Kindly review!

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler at bio.logis Genetic Information Management GmbH commented
Status: Needs review » Reviewed & tested by the community

Perfect, thank you!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Issue summary: View changes
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Issue summary: View changes

Sorry, I misread the patch :)

amit.drupal’s picture

amit.drupal CreditAttribution: amit.drupal as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented

Patch Is beautifully work.

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T commented
Issue summary: View changes
FileSize
Drupal Doc Comment.png67.79 KB

Hi guys.

I applied the patch, study and looks good for me.

Thank you very much @jaykandari.

Good Work.

Regards.

amit.drupal’s picture

amit.drupal CreditAttribution: amit.drupal as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
FileSize
before-apply-patch.png69.15 KB
after-apply-patch.png60.59 KB

After Review Patch Submit Screenshot.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

  • Gábor Hojtsy committed 4e7ec88 on 8.3.x 
    Issue #2865295 by JayKandari: $menuLinkManager property on MenuLinkTree...

  • Gábor Hojtsy committed 92cc819 on 8.4.x 
    Issue #2865295 by JayKandari: $menuLinkManager property on MenuLinkTree...
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Fixed

Looks good, the property was already in use but was indeed undocumented. Thanks all!

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito at Lingotek commented

This change introduced a probably undesired API change. Lauri was right in #5, this change changed visibility from public to protected.
I'm quite sure we don't want to revert this, but worth to notice that we may want a change record?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented

Are undocumented object properties (which are consequentially public) part of the API as we define it? That sounds accidental :)

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito at Lingotek commented

Good call, thanks :-)

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito at Lingotek commented

Good call, thanks :-)

Status: Fixed » Closed (fixed)

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