Problem/Motivation
Follow up #2729597: [meta] Replace \Drupal with injected services where appropriate in core
Proposed resolution
Replace all of them with IoC injection where possible
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #42 | interdiff_37-42.txt | 1.84 KB | mukesh88 |
| #42 | 3123207-42.patch | 5.3 KB | mukesh88 |
| #41 | interdiff_37-41.txt | 3.64 KB | mukesh88 |
| #41 | 3123207-41.patch | 3.18 KB | mukesh88 |
| #40 | Screenshot 2023-02-17 at 10.10.47 AM.png | 344.14 KB | mukesh88 |
Issue fork drupal-3123207
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
ankitsingh0188Comment #4
ankitsingh0188Comment #5
ankitsingh0188Comment #6
junglePer the parent issue, rescoping this to do it for non-test code.
So i have to set this back to NW, sorry for the change in the middle!
Comment #7
hardik_patel_12 commentedList of place where we can inject injection if possible
Kindly review a patch.
Comment #8
jungleI do not think we need a new trait here. ContainerInjectionInterface might help. Tests did not pass.
Comment #12
saphemmy commentedComment #15
ankitsingh0188Comment #16
ankitsingh0188Comment #17
ankitsingh0188Comment #18
ankitsingh0188Comment #19
ankitsingh0188Comment #20
jungle@ankitsingh0188 thanks for working on this.
See https://stackoverflow.com/questions/18142870/git-error-fatal-corrupt-pat... for the error, maybe you should try the git CLI to get the patch or see https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa...
Also, please see #3123210: Replace non-test usages of \Drupal::theme() with IoC injection as a good example.
Comment #21
jungleThose are not "Using IoC as possible". They are almost the same. Again see fixed issue #3123210: Replace non-test usages of \Drupal::theme() with IoC injection as the example.
Comment #22
jungleComment #23
_pratik_Try this patch.
Thanks
Comment #24
_pratik_Comment #25
ankitsingh0188Comment #26
jungleThe patch is fine now, but the patch itself did not address this issue. Please see the one I linked in #21 as an example, or read the parent issue.
Comment #27
ankitsingh0188I think the patch #25 will address the issue as mentioned in #21
Comment #28
jungleOr see the commit of the issue #3123210: Replace non-test usages of \Drupal::theme() with IoC injection as the example
Comment #29
jungle>I think the patch #25 will address the issue as mentioned in #21
Nope, see #26
Comment #30
ankitsingh0188@jungle I have updated the patch and using IoC wherever possible.
Comment #31
jungleThis is fine. it's constructor property promotion introduced in PHP8, let's adopt it in this issue. and hopefully, it gets applied to other sister issues.
A change record link is expected. Tagging needs CR.
Comment #32
ankitsingh0188I've updated the patch with the new CR.
Comment #33
ankitsingh0188Comment #34
ankitsingh0188Comment #35
ankitsingh0188Comment #36
ankitsingh0188Comment #37
ankitsingh0188Comment #38
jungleHi, @ankitsingh0188. I will work on this.
Comment #39
jungleUnassign myself, #37 may be good for review.
Comment #40
mukesh88 commentedAfter applying the patch #37 site is down . After adding some changes in patch #40 site is working
Comment #41
mukesh88 commentedRecreate patch after failed #40
Comment #42
mukesh88 commentedOops!! missing something in patch.