Another part of #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED, bunch of seldomly used on*Create|Update|Delete methods, this is a fairly small patch.

Still need to write the legacy tests similar to #3023981: Add @trigger_error() to deprecated EntityManager->EntityRepository methods

CommentFileSizeAuthor
#4 entity-manager-listener-3028656-4.patch26.97 KBberdir
#2 entity-manager-listener-3028656-2.patch22.17 KBberdir

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new entity-manager-listener-3028656-2.patch22.17 KB

Status: Needs review » Needs work

The last submitted patch, 2: entity-manager-listener-3028656-2.patch, failed testing. View results

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new entity-manager-listener-3028656-4.patch26.97 KB

Missed a few.

martin107’s picture

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

a) The idea behind this issue is good.

b) After a visual scan of the patch - nothing extraneous, all code changes have a clear intent, everything implemented well

c) There are a couple of modifications, for example ModuleInstaller.php that make use of \Drupal::service() where I would like to see direct injections. But after a little reflections I can why this is the best thing to do as there is a @todo notices marking the code as soon to be changes by xyz and so the larger code block will soon be refactored away.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Thanks for the review. Note this will conflict with #3023981: Add @trigger_error() to deprecated EntityManager->EntityRepository methods and I'd prefer for that to get in first as it unblocks other issues.

DI for ModuleHandler is very complex and it's not visible from the patch context, but $entity_manager isn't injected either. So using DI there is IMHO not in scope of this issue.

  • alexpott committed 28bf970 on 8.7.x 
    Issue #3028656 by Berdir, martin107: Add @trigger_error() to deprecated...

Berdir credited alexpott.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed

Looks like this was committed, didn't conflict after all :)

Status: Fixed » Closed (fixed)

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