drupal_container() is deprecated, and all calls in the translation_entity 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: Replace calls to drupal_container()->get('module_handler') service with Drupal::moduleHandler())
This task a part of #2001206: Replace drupal_container() with Drupal::service()
Comment | File | Size | Author |
---|---|---|---|
#22 | content-translation-2014033-22.patch | 2.58 KB | hussainweb |
#18 | translation-entity-2014033-18.patch | 2.49 KB | hussainweb |
#14 | translation-entity-2014033-14.patch | 3.32 KB | kgoel |
#14 | interdiff.txt | 841 bytes | kgoel |
#4 | translation-entity-2014033-4.patch | 3.31 KB | kgoel |
Comments
Comment #1
kgoel CreditAttribution: kgoel commentedComment #2
kgoel CreditAttribution: kgoel commentedComment #3
ddrozdik CreditAttribution: ddrozdik commentedWe should use \Drupal::request() instead \Drupal::service('request').
Comment #4
kgoel CreditAttribution: kgoel commentedComment #6
Berdir#4: translation-entity-2014033-4.patch queued for re-testing.
Comment #7
dcam CreditAttribution: dcam commented#4 looks good to me. I didn't find any more uses of drupal_container() in translation_entity.
Comment #8
dcam CreditAttribution: dcam commentedSorry, I was mistaken. $this->container->get() should be used instead of \Drupal::service in tests.
Comment #9
kgoel CreditAttribution: kgoel commented@dcam please see this - https://drupal.org/node/2014009#comment-7518137
Comment #10
dcam CreditAttribution: dcam commentedYes. That comment is what made me set the status back to NW. Were you referring me to that post due to my mistake or to my retraction?
Comment #11
kgoel CreditAttribution: kgoel commented@dcam - not pointing for any mistake but referred that link because @DmitryDrozdik said to use $this->container->get() instead \Drupal::service in tests.
Comment #12
dcam CreditAttribution: dcam commented@kgoel - Ok, that's cool. Thank you for trying to help me out.
Comment #13
kgoel CreditAttribution: kgoel commented@dcam - ah, nevermind
Comment #14
kgoel CreditAttribution: kgoel commentedComment #15
ddrozdik CreditAttribution: ddrozdik commentedpatch #14 looks good, if bot status will be green - RTBC
Comment #16
dcam CreditAttribution: dcam commentedAgreed, #14 is RTBC.
Comment #17
webchickUnfortunately, this no longer applies for me.
Comment #18
hussainwebRerolling the patch. As a matter of fact, there is no change against the EntityTranslationSyncImageTest.php file anymore, as the latest tree already has that change.
Comment #19
plachThanks!
Comment #20
alexpottSo the module has been renamed in #2024867: Rename translation_entity to content_translation
So this needs a reroll...
Comment #21
Gábor HojtsyComment #22
hussainwebHere is the reroll.
Comment #23
Gábor HojtsyThis should inject the request object properly if not already. Instead of using \Drupal direct.
Comment #24
hussainwebI am thinking about how to inject the object.
My first thought was to check the caller. I see that this function is called through Form API's #entity_bundles. These functions are called with only those four parameters:
On the other hand, this can be injected in the object itself when the '#entity_bundles' is defined, but the documentation for Drupal::request() says this:
The last way that I can see is the recommended method in the documentation for Drupal::request().
However, I am not entirely sure how to go about this. If someone can point me in the right direction, I can take it up, or at least I hope this analysis proves useful.
Comment #25
plachI think we may want to avoid properly injecting the request since that piece code is going to be killed as soon as it is feasible and replaced with plain access to entity fields, hence we won't depend on the request anymore. Thanks!
Comment #26
alexpottCommitted fd3884c and pushed to 8.x. Thanks!
Comment #27
Gábor HojtsyRetroactive tagging.