Deprecate Drupal\locale\Locale and its one method

Yes, we can remove the Drupal\locale\Locale::config() method because in files such as core/modules/locale/locale.bulk.inc and core/modules/locale/tests/src/Functional/LocaleConfigTranslationImportTest.php, the function of the injected service is directly called instead of going through the config() function. Therefore, it's more efficient to directly utilize the injected service in these files.

Keshav Patel → made their first commit to this issue’s fork.

Replaced Drupal\locale\Locale::config() with \Drupal::service('locale.config_manager'), Verified the Checks Using core/scripts/dev/commit-code-check.sh --cached (All Checks Passed). Please Review

Don't see the deprecation.

Deprecated the config() method.

> Deprecated the config() method.

Deprecating doesn't meant removing it -- we have to keep it in, and add a trigger_error() to it so that any callers are told that they need to updated their code.

Deprecation isn’t correct. Recommend looking through the repo for examples of deprecation format

I modified the documentation according to Drupal deprecation policy and added its test.
Please review it.

The test fails, but it doesn't seem to be related to the changes.
https://git.drupalcode.org/issue/drupal-3422915/-/pipelines/133015/test_...⚡%EF%B8%8F%20PHPUnit%20Unit

Seems small enough to deprecate in 10.3 for removal in 11.

Will need a change record and the link points to that. Can be simple and would search the change record list for other deprecation examples for to write one (usually just a sentence or two)

And for the random failure would try and pull the latest 11.x code into this branch. That’s resolved it on other issues.

Good work with the test!

I modified the deprecated version to 10.3 and removal version to 11.0.

I could not create change records because I have no permissions.

On this ticket whaat happens if you click add change notice?

If that doesn’t work will try and help on Monday

> pull the latest 11.x code into this branch

Please rebase rather than merge!

I made a mistake in rebase once, so I force pushed the pre-rebase, updated the 11.x branch of the repository to the latest, and rebased the branch to on top of that.

Thank you for confirming my account.
I could create a new change records.
https://www.drupal.org/node/3437110 →

Deprecation CR looks good.

Thanks for updating link in code too.

Thanks for working on this everyone!

Some unrelated change snuck into the MR - probably due to an IDE - we need to remove that. Also I don;t think this issue should be introducing a camel cased variable everywhere. I
think we should replace Locale::config() we \Drupal::service('locale.config_manager') and be done.

Also we need to add a deprecation to the class body so that we can safely remove the class in 11.x. I. think this is fine here because this in not a plugin and therefore won't trigger unexpected deprecations only good ones.

- Added documentation about the deprecation of the Locale class.
- Removed unnecessary periods.
- About using $localConfigManager as a temporary variable of Locale::config()
- Removed $localConfigManager if it was only used once.
- Renamed to $locale_config_manager if it was used more than once.
Please review it

Deprecation seems good now. Marking this so we don't miss the 10.3 boat.

If we're doing this for removal in 11.0.0, we should also have an 11.x-specific MR here that does the removal directly without the bc layer / test coverage etc. The existing MR should be re-targeted/rebased for 10.3.x

Haven't been asking for this previously, but we're adding new deprecations nearly as quickly as we're removing them still, so think we need to start using that workflow.

Easier to make the current MR for 11.x the one without the deprecation, and make a new branch and MR for 10.3 with the current code.

Looks ready again

Yes, Issues are looking Fixed now

alexpott → changed the visibility of the branch 3422915-10.2.deprecate-drupallocalelocale to hidden.

alexpott → changed the visibility of the branch 11.x to hidden.

I've fixed up the 10.3.x MR so that the test is correct - this does not return a \Drupal\Core\Config\ConfigManager object it returns a \Drupal\locale\LocaleConfigManager object.

Unfortunately the 11.x MR does not do the removal so that needs work.

Also we use \Drupal::service() over Drupal::service()

> This file needs to be removed - this is the 11.x branch.

Ok something went WTF with my branches. I'll fix.

I've fixed the file removals in the 11.x MR. I've no idea how they got lost / and maybe on the wrong branch!

I'm confused about whether there's anything else to fix -- @alexpott you seem to have applied the suggested changes to both branches?

Rebased the 10.3.x MR as the failure appeared to be a random Umami bug.

Believe all feedback has been addressed

Committed dc9a4c5 and pushed to 10.3.x. Thanks!
Committed 4c93088 and pushed to 11.x. Thanks!

quietone → closed merge request !7667

quietone → closed merge request !6948