drush icon indicating copy to clipboard operation
drush copied to clipboard

Drush - Import multiple custom translation po files

Open Sergey-Orlov opened this issue 4 years ago • 12 comments

This adds new drush command to import multiple custom translation po files from directory, as described in #4235

Sergey-Orlov avatar Nov 13 '19 04:11 Sergey-Orlov

We are lacking a locale maintainer at the moment so I apologize that nobody from Drush core team is reviewing this. We need a review from outside the team, as next step.

weitzman avatar Dec 03 '19 20:12 weitzman

Reviewed and tested on real project, all works fine. Can be merged as-is.

alexkras avatar Jan 23 '20 08:01 alexkras

@weitzman what need to do to speed up merge of this changes? thx.

alexkras avatar Jan 23 '20 11:01 alexkras

Thanks for the review.

I'm hesitant to add more locale functionality while we have no maintainer for it. Ideally someone maintains these commands outside of Drush core for now.

weitzman avatar Jan 24 '20 03:01 weitzman

I'd say it needs tests and then it could be a locale console command patch in core

andypost avatar Jan 27 '20 17:01 andypost

FYI we have been using this patch in production for years now without any issue

davidferlay avatar Apr 09 '21 10:04 davidferlay

Maybe we can consider making the directory argument an option? That way, you could set a sitewide default directory through a Drush config file. If you guys are on board with this idea I wouldn't mind implementing it.

DieterHolvoet avatar Jan 31 '22 12:01 DieterHolvoet

Hi @DieterHolvoet , Making the directory argument an option would require to have a default value as fallback (because user can then omit this input, as it is optional). So i'd say, why not providing we figure out a meaningful default value for it.

WDYT ?

davidferlay avatar Jan 31 '22 13:01 davidferlay

Or we could add an extra check in code to make sure the option is present? I would probably use ../translations, relative to the Drupal docroot. In that case we should also support passing relative paths.

DieterHolvoet avatar Jan 31 '22 13:01 DieterHolvoet

Just made a bit of research and discovered that "option" may not necessarily be "optional" (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02)

We could add an extra check in code to make sure the option is present

++ as long the helper reflects it

We should also support passing relative paths

I agree that this would be a welcome addition

davidferlay avatar Jan 31 '22 13:01 davidferlay

The the decision about merging and maintaining this falls to the locale maintainer, @Sutharsan. This PR would need tests and needs to target 11.x branch.

weitzman avatar Jan 31 '22 16:01 weitzman

Okay, before I continue work on this PR I'll wait for their go ahead.

DieterHolvoet avatar Jan 31 '22 16:01 DieterHolvoet