Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
locale_storage() has been replaced with \Drupal::service('locale.storage');
https://drupal.org/node/2016569, change notice for locale_storage().
Comment | File | Size | Author |
---|---|---|---|
#8 | 2020343-diff-1-8.txt | 1.32 KB | vijaycs85 |
#8 | 2020343-locale_storage-removal-8.patch | 6.78 KB | vijaycs85 |
#6 | 2020343-locale_storage-removal-6.patch | 6.74 KB | vijaycs85 |
#6 | 2020343-diff-1-6.txt | 575 bytes | vijaycs85 |
#1 | 2020343-locale_storage-removal-1.patch | 6.72 KB | vijaycs85 |
Comments
Comment #1
vijaycs85Initial patch...
Comment #2
YesCT CreditAttribution: YesCT commentedSo you're injecting it.
Looks like you got all of the locale_storage()'s.
---
I get the rest of this, except for this bit in the tests. Is it like this because it's a test?
Comment #3
vijaycs85sorry don't get your question here.. but there can be 2 questions out of this line :)
1. why aren't we using \Drupal::service('locale.storage) - Yes in test cases we use it's own container.
2. why assigning in ->localStorage, we can call
$this->container->get('locale.storage');
all the place where we need it. but thought would be nice to have as property.Comment #4
YesCT CreditAttribution: YesCT commentedOK.
I had started to take out locale_storage() and inject it while looking at cleaning up the manage form issue, but thought it should be it's own issue. Then I saw this issue already. Most of this is the same as what I had started, but probably better.
I talked with @vijaycs85 in irc too, to clarify some things about this patch.
I also see similar patterns used else where.
I'm going to run this and #2020347: Temporary fix for config_translation_enter_context until language context becomes an option, and if the tests pass, I think they are ok to commit, so that we can unbreak the config translation head. Then we can move on and try and improve things from there.
[update] Not all tests are passing, so I'll wait for that or word from Gabor.
Comment #5
Gábor HojtsyThis looks like a good code change as-is. Some minor notes:
Are we supposed to invoke this as \Drupal...? Not Drupal:: (without backslash)?
Would be good to qualify what storage are these? :) Eg. "String translation storage object" for the variable docs and argument.
Comment #6
vijaycs85As explained by @YesCT on IRC, we need to have \Drupal:: in class and Drupal:: in procedural code. so updating just comment from #5
Comment #7
Gábor HojtsyString translation storage object only changed at one place instead of 3.
Comment #8
vijaycs85Fixing all 3 instances.
Comment #9
Gábor HojtsyThanks, committed this one!
Comment #10.0
(not verified) CreditAttribution: commentedadding link to change notice.