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

Comments

tstoeckler created an issue. See original summary.

jaykandari’s picture

jaykandari
Primary language Hindi
commented
Assigned: Unassigned » jaykandari
jaykandari’s picture

jaykandari
Primary language Hindi
commented
Assigned: jaykandari » Unassigned
Status: Active » Needs review
StatusFileSize
new menulinkmanager-2865295-3.patch597 bytes

Changes applied. Kindly review!

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Reviewed & tested by the community

Perfect, thank you!

lauriii’s picture

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

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

Sorry, I misread the patch :)

amit.drupal’s picture

amit.drupal commented

Patch Is beautifully work.

renatog’s picture

renatog commented
Issue summary: View changes
StatusFileSize
new 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 commented
StatusFileSize
new before-apply-patch.png69.15 KB
new 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
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
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 🇪🇺
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
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 🇪🇺
commented

Good call, thanks :-)

penyaskito’s picture

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

Good call, thanks :-)

Status: Fixed » Closed (fixed)

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