Updated: Comment 0

Problem/Motivation

Both the tempstore as well as the image toolkits are defined by actual modules so they should not live in core.services.yml

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#21 interdiff.txt1.2 KBdawehner
#21 services-2100313-21.patch3.31 KBdawehner
#19 services-2100313-19.patch2.11 KBdawehner
#16 services-2100313-16.patch2.63 KBpwolanin
#10 services-2100313-10.patch14.75 KBpwolanin
#10 2100313-7-10.increment.txt1.76 KBpwolanin
#7 services-2100313-7.patch12.99 KBdawehner
#1 2100313.patch2.04 KBdawehner

Comments

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 2100313.patch2.04 KB

.

pwolanin’s picture

pwolanin commented

Looks reasonable, is there any reason we need those early in install?

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review

UPdated.

Status: Needs review » Needs work

The last submitted patch, 2100313.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review

#1: 2100313.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 2100313.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new services-2100313-7.patch12.99 KB

Let's better move the image toolkit manager to core

pwolanin’s picture

pwolanin commented

Looks better. Just got a +1 from Crell also to the concept of this change.

Status: Needs review » Needs work

The last submitted patch, services-2100313-7.patch, failed testing.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new 2100313-7-10.increment.txt1.76 KB
new services-2100313-10.patch14.75 KB

The fatal error in the test is:
"ResponseText: A fatal error occurred: The service definition "user.tempstore" does not exist."

Easy fix - we now need to enable user module for those tests.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

#10: services-2100313-10.patch queued for re-testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Component: toolbar.module » base system
dawehner’s picture

dawehner
Primary language German
commented

#2048827: Move Image toolkit API from system.module to Drupal\Core

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

#10: services-2100313-10.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, services-2100313-10.patch, failed testing.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new services-2100313-16.patch2.63 KB

rebased to resolve the conflicts.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

16: services-2100313-16.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 16: services-2100313-16.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new services-2100313-19.patch2.11 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, 19: services-2100313-19.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs work » Needs review
StatusFileSize
new services-2100313-21.patch3.31 KB
new interdiff.txt1.2 KB

Fixes the fails.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +API clean-up

Thanks!

That service being registered in core.services.yml always confused me. Since User module is required, there's no functional difference here, just the registration is moved.

catch’s picture

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

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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