Suggested commit message

Issue #2301319 by pwolanin, dawehner, Wim Leers, effulgentsia: MenuLinkNG part5: Remove dead code; and party!

Problem/Motivation

After #2301317: MenuLinkNG part4: Conversion goes in there will be some dead code left in core - this patch just removes it.

Proposed resolution

Remaining tasks

User interface changes

API changes

Original report by @effulgentsia

Final chunk after #2301317: MenuLinkNG part4: Conversion. This removes the old menu_link module and dead procedural functions from menu.inc.

Comments

pwolanin’s picture

Status: Postponed » Needs review
FileSize
612.91 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,964 pass(es). View
128.8 KB

Testing the full patch with removal after fixing 8.x conflicts

pwolanin’s picture

FileSize
615.32 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,982 pass(es), 0 fail(s), and 50 exception(s). View
128.8 KB

Here's a new one including fixups at step 1

Status: Needs review » Needs work

The last submitted patch, 3: 2301319-3.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
617.38 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,817 pass(es). View

Here's the full patch

cilefen’s picture

FileSize
288.89 KB

The xhprof runs:

53c29adf192f7.localhost@_drupal8x_menu-bench.xhprof is a load of /menu-bench from https://github.com/pwolanin/menu_bench.git on HEAD.
53c2d8e0670a6.localhost@_drupal8x_menu-bench.xhprof is the same with last patch.

xjm’s picture

Issue summary: View changes
xjm’s picture

Priority: Normal » Critical
pwolanin’s picture

FileSize
628.81 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,991 pass(es). View

Here's the current full patch

pwolanin’s picture

FileSize
403.97 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2301319-10.patch. Unable to apply patch. See the log in the details link for more information. View

Current full patch after part1 is committed.

Status: Needs review » Needs work

The last submitted patch, 10: 2301319-10.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
474.26 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,283 pass(es). View

Rebases cleanly, not sure why it doesn't apply

pwolanin’s picture

FileSize
403.98 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,682 pass(es). View

This should be the full remaining patch since parts 1 and 2 are committed.

pwolanin’s picture

FileSize
409.3 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 73,303 pass(es), 15 fail(s), and 2 exception(s). View
128.8 KB

updated combined patch and part5 only

Status: Needs review » Needs work

The last submitted patch, 14: 2301319-14.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
Issue tags: +MenuSystemRevamp
FileSize
408.25 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,057 pass(es). View

with fixes from part4

pwolanin’s picture

Issue summary: View changes
FileSize
408.53 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2301319-17.patch. Unable to apply patch. See the log in the details link for more information. View

Updated full remaining patch

Status: Needs review » Needs work

The last submitted patch, 17: 2301319-17.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
409.67 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,193 pass(es). View

re-rolled full patch

pwolanin’s picture

FileSize
383.93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,924 pass(es). View

new snapshot of the full patch

pwolanin’s picture

FileSize
392.82 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,203 pass(es), 88 fail(s), and 0 exception(s). View
583 bytes
128.73 KB

Updated patch with all changes from part4, and some small added cleanup

Status: Needs review » Needs work

The last submitted patch, 21: 2301319-21.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
393.4 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,020 pass(es), 8 fail(s), and 3 exception(s). View

With part4 system controller fixes

Status: Needs review » Needs work

The last submitted patch, 23: 2301319-23.patch, failed testing.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
398.34 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,039 pass(es). View

adding in fixes from part4

pwolanin’s picture

FileSize
128.73 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 74,964 pass(es). View

last part!

Final patch after #2301317: MenuLinkNG part4: Conversion

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs change record updates

Looks good! Just deletions of functions in menu.inc, and the old menu_link module, whose .info.yml was already deleted in part 4.

After this lands, we should go back to the part 4 change records, and ensure these menu.inc deletions are referenced in there:

-const MENU_MAX_DEPTH = 9;
-function menu_set_active_menu_names($menu_names = NULL) {
-function menu_get_active_menu_names() {
-function menu_link_get_preferred($path = NULL, $selected_menu = NULL) {
-function menu_link_rebuild_defaults() {
-function menu_load_links($menu_name) {
-function menu_delete_links($menu_name) {
xjm’s picture

Thanks @effulgentsia! I checked and these are covered in https://www.drupal.org/node/2302069 with updates I wrote yesterday:
const MENU_MAX_DEPTH = 9;
function menu_load_links($menu_name) {
function menu_delete_links($menu_name) {

Covered in previous CR https://www.drupal.org/node/2240003
function menu_link_get_preferred($path = NULL, $selected_menu = NULL)

No references I found:
function menu_set_active_menu_names($menu_names = NULL) {
function menu_get_active_menu_names() {
function menu_link_rebuild_defaults() {

xjm’s picture

Issue tags: +beta blocker

Tagging the child issues retroactively.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

MenuHierarchy still uses MENU_MAX_DEPTH.

pwolanin’s picture

Status: Needs work » Needs review
FileSize
131.24 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,315 pass(es). View
2.51 KB

I think we can just remove that class - the menu tree storage should not be access by Views.

xjm’s picture

Issue tags: +VDC

We discussed this a bit in IRC. As nothing is using this handler in core and there is no test coverage for it, @damiankloip and I are okay with simply removing the handler, at least here. I've left a message asking @dawehner to make the call as to whether we should consider this a regression that needs a followup.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

I also left a comment in #1805996-71: [META] Views in Drupal Core. Even if we need that plugin back, I think that should be a followup rather than hold up this patch, as this plugin is completely broken in HEAD as-is (there's no {menu_links} table to join to). Therefore, back to RTBC if green.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Repeat after me: yay!!! Thanks everyone.

  • Dries committed c77851c on 8.0.x
    Issue #2301319 by pwolanin, effulgentsia: MenuLinkNG part5: Remove dead...

Status: Fixed » Closed (fixed)

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