Comments

Hardik_Patel_12 created an issue. See original summary.

hardik_patel_12’s picture

StatusFileSize
new3.09 KB

Kindly review a new patch.

hardik_patel_12’s picture

Assigned: hardik_patel_12 » Unassigned
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 3107001-2.patch, failed testing. View results

hardik_patel_12’s picture

StatusFileSize
new3.13 KB
new705 bytes

Kindly review a new patch.

hardik_patel_12’s picture

Status: Needs work » Needs review
longwave’s picture

Thank you for these patches, but I think that making these changes per file is going to take a long time and a lot of effort.

In #2729597: [meta] Replace \Drupal with injected services where appropriate in core it was suggested that we perhaps make these sorts of replacement on a per service basis - I wonder if you have read that issue and could help out with this type of change over there?

alexpott’s picture

@longwave good call on scope. I'm not sure about the per service basis as classes often need multiple injections. Here's another suggestion...

@Hardik_Patel_12 thanks for filing this patch. Fixing dependency inject on a class by class basis is going to be quite painful. Please read https://www.drupal.org/core/scope - I think a good scope here would be to combine this with #3107003: \Drupal calls should be avoided in classes, use dependency injection instead in core/modules/update/src/Form classes and do all the injection necessary in core/modules/update/src/Form - there are three forms to update there.

So I'm going to close this as a duplicate of #3107003: \Drupal calls should be avoided in classes, use dependency injection instead in core/modules/update/src/Form classes as that has further instructions on how to do this and respect our BC promise.