Split from #2102521: Finish converting menu.module to CMI.
Child of #1775842: [meta] Convert all variables to state and/or config systems

core/modules/menu/menu.module includes the following calls to the variable system, which will not be available in Drupal 8.

217 $active_menus = variable_get('menu_default_active_menus', array_keys(menu_get_menus()));
221 variable_set('menu_default_active_menus', $active_menus);

This variable has been partially migrated to system.menu.active_menus_default

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ianthomas_uk’s picture

Status: Active » Needs review
FileSize
1.06 KB
vijaycs85’s picture

Looks good to me. Except a minor coding standard bug.

+++ b/core/modules/menu/menu.module
@@ -209,12 +209,15 @@ function menu_menu_predelete(Menu $menu) {
+    // Prevent the gap left by the removed menu from causing array indices to be saved

Minor: Comment exceeds 80 chars limit and missing period at the end.

btw, foreach-- :), so ianmthomasuk++

vijaycs85’s picture

Also noted that the system_update_8033 trying to update the wrong variable name (i.e. 'active_menus_default'). Can you update system_update_8033 as well please? It should be:

/**
 * Convert active_menus_default variable to config.
 *
 * @ingroup config_upgrade
 */
function system_update_8033() {
  update_variables_to_config('system.menu', array(
    'menu_default_active_menus' => 'active_menus_default'
  ));
}

ianthomas_uk’s picture

New patch addressing #2 and #3

vijaycs85’s picture

Thanks @ianmthomasuk.

+1 from my side to RTBC.

catch’s picture

Status: Needs review » Reviewed & tested by the community
Berdir’s picture

Component: content_translation.module » menu.module

Wrong component?

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

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