drupal_container() is deprecated, and all calls in the breakpoint module need to be replaced with Drupal::service(), except for where the module_handler service is requested, which needs to be replaced with Drupal::moduleHandler() (see #1957154)

CommentFileSizeAuthor
#13 2003260-replace-drupal_container-breakpoint-module-13.patch2.31 KBkgoel
#13 interdiff.txt1.28 KBkgoel
#8 2003260-replace-drupal_container-breakpoint-module.patch1.67 KBddrozdik
#5 2003260-replace-drupal_container-breakpoint-module.patch1.63 KBddrozdik
#3 2003260-3.patch1.38 KBebeyrent
#2 2003260-2.patch1.78 KBebeyrent
#1 2003260-1.patch1.78 KBebeyrent

Comments

ebeyrent’s picture

ebeyrent commented
StatusFileSize
new 2003260-1.patch1.78 KB
ebeyrent’s picture

ebeyrent commented
StatusFileSize
new 2003260-2.patch1.78 KB
ebeyrent’s picture

ebeyrent commented
Status: Active » Needs review
StatusFileSize
new 2003260-3.patch1.38 KB
ddrozdik’s picture

ddrozdik commented

patch #3 looks good, but last time was lots of commits, and need re-test.

ddrozdik’s picture

ddrozdik commented
Assigned: ebeyrent » ddrozdik
Issue tags: +CodeSprintUA
StatusFileSize
new 2003260-replace-drupal_container-breakpoint-module.patch1.63 KB

Created a new patch for whole task.
Removed @todo from install file, because that task already closed.
Also the best way to use moduleHandler via Drupal::moduleHandler() not Drupal::service('module_handler') as in patch #2

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Needs review » Reviewed & tested by the community

#5 looks good
RTBC

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

curl https://drupal.org/files/2003260-replace-drupal_container-breakpoint-module.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1665  100  1665    0     0   2236      0 --:--:-- --:--:-- --:--:--  2910
error: patch failed: core/modules/breakpoint/breakpoint.install:18
error: core/modules/breakpoint/breakpoint.install: patch does not apply
ddrozdik’s picture

ddrozdik commented
Status: Needs work » Needs review
StatusFileSize
new 2003260-replace-drupal_container-breakpoint-module.patch1.67 KB
ddrozdik’s picture

ddrozdik commented
Component: block.module » breakpoint.module
Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

And again.

yesct’s picture

yesct commented
Issue tags: -Needs reroll +RTBC July 1

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

There are other deprecated functions called in breakpoint.module... config() and entity_get_info() which are both wrappers to container services... and have \Drupal equivalents...

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt1.28 KB
new 2003260-replace-drupal_container-breakpoint-module-13.patch2.31 KB
Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

If the bot approves...

Status: Reviewed & tested by the community » Needs work
Issue tags: -WSSCI Conversion, -CodeSprintUA, -RTBC July 1

The last submitted patch, 2003260-replace-drupal_container-breakpoint-module-13.patch, failed testing.

kgoel’s picture

kgoel commented
Status: Needs work » Needs review
Issue tags: +WSSCI Conversion, +CodeSprintUA, +RTBC July 1

#13: 2003260-replace-drupal_container-breakpoint-module-13.patch queued for re-testing.

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

Sigh, testbot.

webchick’s picture

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

Committed and pushed to 8.x. Thanks!

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.