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
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,873 pass(es). View
#1 2419857-1.patch1.22 KBWim Leers
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2419857-1.patch. Unable to apply patch. See the log in the details link for more information. View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

Wim Leers’s picture

Priority: Normal » Major
Status: Active » Needs review
Issue tags: +Configuration system
FileSize
1.22 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2419857-1.patch. Unable to apply patch. See the log in the details link for more information. View

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

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

Issue tags: +Novice
alexpott’s picture

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

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

Status: Needs work » Needs review
FileSize
2.07 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 85,873 pass(es). View

New patch

Wim Leers’s picture

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

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

Nice, yay!

Status: Fixed » Closed (fixed)

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