This is a sub-issue of #1910624: [META] Introduce and complete configuration schemas in all of core.

Problem/motivation

#1866610: Introduce Kwalify-inspired schema format for configuration introduced the idea of config schema. The changelog leads to (hopefully extensive) documentation on the format at http://drupal.org/node/1905070. While there are little cleanups planned for the format overall, the current format is a result of months of back and forths, so it should be perfectly fine to apply it more widely to core.

Proposed solution

Create a configuration schema for menu module.

Schema in place

Schema not yet in place
menu.menu.account.yml
menu.menu.admin.yml
menu.menu.footer.yml
menu.menu.main.yml
menu.menu.tools.yml
menu.settings.yml

CommentFileSizeAuthor
#9 1919184-menu-schema-to-system-9.patch1.4 KBvijaycs85
#5 1919184-menu-schema-5.patch860 bytesnikhilasrani
#5 1919184-diff-2-5.txt410 bytesnikhilasrani
#2 1919184-menu-schema-2.patch880 bytesnikhilasrani
#1 1919184-menu-schema-1.patch611 bytesvijaycs85
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Active » Needs review
FileSize
1919184-menu-schema-1.patch611 bytes

Adding schema file...

nikhilasrani’s picture

nikhilasrani CreditAttribution: nikhilasrani commented
FileSize
1919184-menu-schema-2.patch880 bytes

Re-rolling with below changes as advised in Schema code style:
1. moved from config to config/schema.
2. Updated labels.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented

Thank you for your patch. Just one thing.

+++ b/core/modules/menu/config/schema/menu.schema.ymlundefined
@@ -0,0 +1,29 @@
+  type: mapping
+  label: 'Individual menu settings'

Not sure about this label. Will have a word with @Gabor or @YesCT and RTBC it.

Gábor Hojtsy’s picture

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

I'm not really sure if I have a better suggestion for that label. "Menu" is good as well as it was used above, maybe more useful on the UI at the end.

nikhilasrani’s picture

nikhilasrani CreditAttribution: nikhilasrani commented
FileSize
1919184-diff-2-5.txt410 bytes
1919184-menu-schema-5.patch860 bytes

Thanks @Gábor Hojtsy. Updated label as "Menu"

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Needs review » Reviewed & tested by the community

Thanks @nikhilasrani. RTBC, if comes back green.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Fixed » Needs work

This seems wrong. In fact system module defines the Menu config entity, so menus can exist without menu module. Also menu module is not required, so to have schema coverage for those menus, I think the menu.menu.* should move to system schema.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Status: Needs work » Needs review
FileSize
1919184-menu-schema-to-system-9.patch1.4 KB

Updating menu.menu.* to system.schema.yml.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. Simply moves the relevant schema.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Hm. I guess that make sense. I'm a little leery of putting this into system's schema, but it's true that menu is a totally optional module for tweaking things, whereas the concept of a menu (currently) isn't.

Committed to 8.x, thanks! I'll push once testbot has caught up a bit.

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