Problem/Motivation

locale_translate_batch_import_files() is unused since #1848490: Import translations automatically during installation - and it is broken because the $langcodes is an array but the operator is =.

    $result = \Drupal::database()->select('locale_file', 'lf')
      ->fields('lf', ['langcode', 'uri', 'timestamp'])
      ->condition('langcode', $langcodes)
      ->execute()
      ->fetchAllAssoc('uri');

Proposed resolution

Remove the code and fix the documentation.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3575729

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:

About issue forks

Comments

alexpott created an issue. See original summary.

alexpott opened merge request !14911

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
Related issues: +#3215707: [META] Modernize Locale module
nicxvan’s picture

nicxvan commented

Any chance we can get #3572339: Modernize locale.fetch.inc in first?

I am trying to go through locale as party of the module cleanup and trying to organize them to reduce conflicts.

I created suggestions on the referenced issue for the comment change I'll apply in a bit since those should be made either way there.

nicxvan’s picture

nicxvan commented

locale.fetch.inc is in.

The thing is locale.bulk is the biggest of the files and it's too big I think to do the full conversion in one issue.

  • locale_translate_batch_import_files
  • locale_translate_get_interface_translation_files
  • locale_translate_batch_build
  • locale_translate_batch_import
  • locale_translate_batch_import_save
  • locale_translate_batch_refresh
  • locale_translate_batch_finished
  • locale_translate_file_create
  • locale_translate_file_attach_properties
  • locale_translate_delete_translation_files
  • locale_config_batch_update_components
  • locale_config_batch_build
  • locale_config_batch_update_default_config_langcodes
  • locale_config_batch_set_config_langcodes deprecated
  • locale_config_batch_update_config_translations
  • locale_config_batch_refresh_name deprecated
  • locale_config_batch_finished

Removing just the one function is clean but doesn't reduce the file enough for the remaining to be one more issue either.[

I'd propose we deprecate a few more in this issue:
locale_translate_get_interface_translation_files
locale_translate_file_create
locale_translate_file_attach_properties
locale_translate_delete_translation_files

These are the non batch related functions afaict, I think they could go in a LocaleTranslationFile service.
I can work on this if you agree, but I don't feel super strongly about this so we can just move forward here if you think it should be isolated.

I did search contrib and the function here is not being used.

dcam’s picture

dcam commented

+1 from me for expanding the scope to move the other functions. Moving file management functions to a service makes sense to me. But I also want to note that locale_translate_file_create isn't used anymore either. Its only use is in the contrib drush_language module, which doesn't have a D11 release. Even if we focus on removing unused functions, then that one really seems like it should be in the scope of this issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Task » Bug report

@nicxvan please can we remove this one function? It is broken which is different from needing to be moved - especially in light of #3575678: Locale checks file mtime but this is tricky if the translation files are copied to an image by Docker use a hash of the file instead which is also fixing a bug rather than refactoring code. Also this is a bug fix and not task. Let's not mix refactors and bug fixes.

nicxvan’s picture

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

In that case I think this is ready, I can't rebase through the UI.

godotislate’s picture

godotislate
he/him
commented
Title: locale_translate_batch_import_files() is unused - let's remove it » Remove locale_translate_batch_import_files() because it is unused

godotislate closed merge request !14911

  • godotislate committed 8a94b995 on main 
    fix: #3575729 Remove locale_translate_batch_import_files() because it is...

  • godotislate committed 207e4ae6 on 11.x 
    fix: #3575729 Remove locale_translate_batch_import_files() because it is...
godotislate’s picture

godotislate
he/him
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to main and 11.x. Thanks everyone!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev

Status: Fixed » Closed (fixed)

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