[Translation] Added Lokalise Provider #40927

welcoMattic · Apr 23, 2021

Q	A
Branch?	5.x
Bug fix?	no
New feature?	no
Deprecations?	no
Tickets
License	MIT
Doc PR	symfony/symfony-docs#15310

To follow up on #38475, this PR adds Lokalise Provider.

The todo list to make it ready is:

Apply recent changes that have been made on ProviderInterface and TranslatorBagInterface (we removed the all() and getDomains() method from TranslatorBagInterface)
Add LokaliseProvider to src/Symfony/Bundle/FrameworkBundle/Resources/config/translation_providers.php file
Add Lokalise case to Symfony\Component\Translation\Exception\UnsupportedSchemeException
Move LokaliseProvider and LokaliseProviderFactory from Symfony\Component\Translation\Bridge\Lokalise\Provider to Symfony\Component\Translation\Bridge\Lokalise namespace
Write integration tests by mocking HTTP Responses

The major part of the remaining work concerns tests, I will make it done before the beginning of May.

carsonbot · Apr 23, 2021

Please note that you need squash your commits before this PR can be merged. The maintainer can also squash the commits for you, but then you need to “Allow edits from maintainer” (there is a checkbox in the sidebar of the PR).

Cheers!

Carsonbot

OskarStark

Do you need to adjust UnsupoortedSchemeException like we do for the Notifier?

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

…etter (Nyholm) This PR was merged into the 5.3-dev branch. Discussion ---------- [CI] Sort packages by length to match modified package better | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Given the build error in #40927, I saw that we match "modified packages" wrong. The script things we modified `symfony/translation` rather than the new bridge. This is because we are using a simple [string matchning](https://github.com/symfony/symfony/blob/18658a29a37d6de3e73c95f96fb9e51bf1d5f59d/.github/get-modified-packages.php#L24). If we sort the packages by length, we make sure we match the most detailed (longest) string first. Commits ------- f7a0bd1 [CI] Sort packages by length to match modified package better

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/translation_providers.php

welcoMattic · Apr 28, 2021

Thanks @OskarStark for the quick review, I have not finish the revamp of Lokalise accordingly to the recent changes 😉

welcoMattic · May 5, 2021

status: needs review

CI failures seems unrelated to this PR.

nicolas-grekas

(with easy to fix comments)

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProvider.php

fabpot · May 9, 2021

Tests seem broken.

welcoMattic · May 9, 2021

@fabpot I've just pushed a fix (adding symfony/config as dev dependency of the Lokalise Provider Bridge). It fixes tests on Travis. On AppVeyor, failures seems unrelated

fabpot

LGTM, to be merged after the HOST constant is fixed.

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProviderFactory.php

welcoMattic · May 10, 2021

@fabpot Last comment has been addressed, branch has been rebased, it looks ready 👍

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProviderFactory.php

fabpot · May 10, 2021

Thank you @welcoMattic.

stof · May 10, 2021

@welcoMattic actually, the symfony/config component is necessary when using the xliff loader (which is why it is only a dev dependency in symfony/translation as you are not forced to use xliff). As the Lokalise provider relies on loading xliff, it should have a non-dev requirement on symfony/config

stof · May 10, 2021

@welcoMattic does this provider handle properly the fact that Symfony translation keys are only unique inside a domain, not globally ?

welcoMattic · May 10, 2021

@stof indeed, actually all Translation Providers use XliffFileLoader for loading pulled translations, so I guess I should set symfony/config as hard dependency in Loco, Lokalise, PoEditor and Crowdin Providers?

About handling translation keys by domain and not glabally, I paid attention to handle it in all Providers after your first comment about it.

stof · May 10, 2021

so I guess I should set symfony/config as hard dependency in Loco, Lokalise, PoEditor and Crowdin Providers?

yes, indeed.

welcoMattic · May 10, 2021

@stof fixed here #41155

This PR was merged into the 5.3-dev branch. Discussion ---------- [Translation] Improved Translation Providers | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | Some small improvements on Translation Providers: - move symfony/config from dev dependency to hard dependency for all Bridges as all providers use XliffFileLoader to load pulled translations (cf. #40927 (comment)) - replace all instances of ``` 'headers' => [ 'Authorization' => 'Bearer API_TOKEN', ] ``` with `'auth_bearer' => 'API_TOKEN',` even in tests files. - Fix Lokalise base_uri concatenation Commits ------- 84fd13c Improved Translation Providers

This PR was submitted for the 5.4 branch but it was squashed and merged into the 5.3 branch instead. Discussion ---------- [Translation] Introduce Translation Providers Docs for symfony/symfony#38475, symfony/symfony#40926, symfony/symfony#40927, and symfony/symfony#40947 Ready for first review, but I'm not sure that I've written documentation in the right and all required places. ATM, Translation Providers Bridges packages doesn't exists, so Flex recipes are not created yet. Commits ------- 943a63f [Translation] Introduce Translation Providers

carsonbot added Status: Needs Review Translation labels Apr 23, 2021

Nyholm added this to the 5.3 milestone Apr 23, 2021

Nyholm mentioned this pull request Apr 23, 2021

[CI] Sort packages by length to match modified package better #40928

Merged

OskarStark reviewed Apr 24, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php Outdated Show resolved Hide resolved

welcoMattic force-pushed the feature/lokalise-provider branch 2 times, most recently from c41ea84 to 749cca8 Compare April 27, 2021 16:23

OskarStark reviewed Apr 28, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php Outdated Show resolved Hide resolved

OskarStark reviewed Apr 28, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/translation_providers.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels May 5, 2021

welcoMattic force-pushed the feature/lokalise-provider branch 3 times, most recently from 906af69 to d7a0ad9 Compare May 5, 2021 13:08

carsonbot added Status: Needs Review and removed Status: Needs Work labels May 5, 2021

welcoMattic force-pushed the feature/lokalise-provider branch 2 times, most recently from d434655 to 2399fc7 Compare May 5, 2021 13:18

welcoMattic mentioned this pull request May 5, 2021

[Translation] Introduce Translation Providers symfony/symfony-docs#15310

Merged

welcoMattic force-pushed the feature/lokalise-provider branch from 2399fc7 to 608685f Compare May 6, 2021 20:39

nicolas-grekas approved these changes May 7, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php Outdated Show resolved Hide resolved

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProvider.php Outdated Show resolved Hide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels May 7, 2021

welcoMattic force-pushed the feature/lokalise-provider branch 3 times, most recently from 4fef59d to 31df459 Compare May 7, 2021 16:35

nicolas-grekas added the Ready label May 9, 2021

welcoMattic force-pushed the feature/lokalise-provider branch from 15b6ba2 to 6a5d222 Compare May 9, 2021 16:02

fabpot approved these changes May 10, 2021

View reviewed changes

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProviderFactory.php Outdated Show resolved Hide resolved

welcoMattic force-pushed the feature/lokalise-provider branch 2 times, most recently from ebd9708 to fe0685a Compare May 10, 2021 09:58

fabpot reviewed May 10, 2021

View reviewed changes

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProviderFactory.php Outdated Show resolved Hide resolved

welcoMattic force-pushed the feature/lokalise-provider branch 2 times, most recently from 631b635 to 82179a6 Compare May 10, 2021 10:05

fabpot reviewed May 10, 2021

View reviewed changes

src/Symfony/Component/Translation/Bridge/Lokalise/LokaliseProviderFactory.php Outdated Show resolved Hide resolved

welcoMattic force-pushed the feature/lokalise-provider branch 2 times, most recently from 3b068e8 to 89f9e1d Compare May 10, 2021 10:07

fabpot approved these changes May 10, 2021

View reviewed changes

Added Lokalise Provider

022d828

welcoMattic force-pushed the feature/lokalise-provider branch from 89f9e1d to 022d828 Compare May 10, 2021 10:15

fabpot merged commit 0b66008 into symfony:5.x May 10, 2021

welcoMattic deleted the feature/lokalise-provider branch May 10, 2021 11:51

welcoMattic mentioned this pull request May 10, 2021

[Translation] Improved Translation Providers #41155

Merged

fabpot mentioned this pull request May 12, 2021

Release v5.3.0-BETA4 #41196

Merged

chr-hertel mentioned this pull request Jan 30, 2023

[Translation] Phrase translation provider #49142

Closed

Search code, repositories, users, issues, pull requests...

Uh oh!

[Translation] Added Lokalise Provider #40927

[Translation] Added Lokalise Provider #40927

Uh oh!

Conversation

welcoMattic commented Apr 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carsonbot commented Apr 23, 2021

Uh oh!

OskarStark left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

welcoMattic commented Apr 28, 2021

Uh oh!

welcoMattic commented May 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fabpot commented May 9, 2021

Uh oh!

welcoMattic commented May 9, 2021

Uh oh!

fabpot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

welcoMattic commented May 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fabpot commented May 10, 2021

Uh oh!

stof commented May 10, 2021

Uh oh!

stof commented May 10, 2021

Uh oh!

welcoMattic commented May 10, 2021

Uh oh!

stof commented May 10, 2021

Uh oh!

welcoMattic commented May 10, 2021

Uh oh!

Uh oh!

welcoMattic commented Apr 23, 2021 •

edited

Loading

welcoMattic commented May 5, 2021 •

edited

Loading

welcoMattic commented May 10, 2021 •

edited

Loading