Problem/Motivation

The responsive image config entity type's schema reimplements a lot of what the config_entity type already provides. Of course, that type was only added after responsive image module's schema was created, so that's understandable.

Proposed resolution

Use the config_entity type.

Remaining tasks

Do it.

User interface changes

None.

API changes

TBD (nothing actually changes, but the types do change, so…)

CommentFileSizeAuthor
#7 2419857.7.patch2.07 KBalexpott
#1 2419857-1.patch1.22 KBwim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Priority: Normal » Major
Status: Active » Needs review
Issue tags: +Configuration system
StatusFileSize
new 2419857-1.patch1.22 KB

Not sure about the priority, but feels like this is at least major.

Status: Needs review » Needs work

The last submitted patch, 1: 2419857-1.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Needs reroll

Hehe, oops, this patch is relative to #2260061: Responsive image module does not support sizes/picture polyfill 2.2. Will reroll against HEAD later unless somebody beats me to it; easy to reroll!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Novice
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Responsive image should use the config_entity type, can then be simpler » Responsive image and View mode schemas should use the config_entity type, can then be simpler
Category: Task » Bug report

This is a bug tbh. Really nice find. I wonder if any other config entities do this. Yep core.entity_view_mode.*.* - it is the only other.

Let's fix that here too.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: -D8 upgrade path

We discussed the upgrade path tag, I don't think it is needed, nothing will break, the order might change if you export them again, and with #2361775: Third party settings dependencies cause config entity deletion, there will be a new top level key, but that is just an adition.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 2419857.7.patch2.07 KB

New patch

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll, -Novice +Quickfix

So… #2361775: Third party settings dependencies cause config entity deletion landed. Not sure if that affects anything.

This is so braindead simple that I'm forced to RTBC.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Very nice one. Committed/pushed to 8.0.x, thanks!

  • catch committed 58e0453 on 8.0.x 
    Issue #2419857 by Wim Leers, alexpott: Responsive image and View mode...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Nice, yay!

Status: Fixed » Closed (fixed)

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