Problem/Motivation

Let's convert and deprecate:

  • locale_translation_use_remote_source - Inline this logic
  • locale_string_is_safe - Move to LocaleXss::stringIsSafe
  • locale_translatable_language_list - Move to LocaleLanguages
  • locale_js_translate - Move to LocaleJs
  • locale_translation_language_table - move to form class
  • locale_is_translatable - Move to LocaleLanguages

Steps to reproduce

Open locale.module

Proposed resolution

Convert them to the classes in the Motivation section

Remaining tasks

Review
Decide if we need to do BC for the constructor changes.

This adds a new parameter or two to many classes, some are hooks or event subscribers which we generally don't need to do BC for.
Some are forms, which we sometimes provide for and a few services.
Here is the list of classes not counting new ones.

  • 5 services
  • 5 forms
  • 5 hook classes
  • 1 event subscriber

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3616277

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

nicxvan created an issue. See original summary.

nicxvan’s picture

nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

I still need to add the constructor deprecations, but let's see how tests look.

nicxvan’s picture

Status: Active » Needs review

I think this is ready for review now for naming and placement.

I didn't do constructor BC, I'm not sure we need to do it for forms.

There is no contrib extending TranslateFormBase.

nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

Issue summary: View changes
berdir’s picture

Status: Needs review » Needs work
nicxvan’s picture

Status: Needs work » Needs review
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new667 bytes

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

nicxvan’s picture

Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

Github is down so the tests are failing but the changes are good for review.

nicxvan’s picture

Issue tags: -no-needs-review-bot

Pretty sure there nightwatch test is random.

Fixed a bunch of phpstan issues introduced on the last rename.

Should be good again.

daffie’s picture

Status: Needs review » Needs work
Issue tags: +Needs change record updates, +Needs issue summary update

The PR looks good to me. Almost RTBC. Just a couple of nitpicks.
Could you add in the CR code examples with before and after for what has changed.
The IS needs to be updated for the deprecations.

nicxvan’s picture

Issue summary: View changes
nicxvan’s picture

I updated the IS and the CR, thanks, those had gotten out of date.

I also tested both : and :: for the form callback, both work.

nicxvan’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record updates, -Needs issue summary update

This should be ready for review again thanks!

nicxvan’s picture

Issue summary: View changes
berdir’s picture

Status: Needs review » Needs work

See my comment.

nicxvan’s picture

Status: Needs work » Needs review

Thanks for the clarifications, I've updated it and it's now green again!

berdir’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me now. locale_translation_use_remote_source() being inlined is currently quite verbose, but that's largely due to the long config and constant. We have a separate issue to convert the constants, the different values of that setting could be an enum, that could already make it significantly more readable.

I contributed quite a bit with specific suggestions on naming and so on, if not actual code, but we also have an "Almost RTBC" in #14 from @daffie, and we answered those nitpicks.