php-translation / translator

Services to translate strings, Like GoogleTranslate and BingTranslate
MIT License
25 stars 16 forks source link

Support mutiple strings in same request #14

Closed Korbeil closed 4 years ago

Korbeil commented 6 years ago

Issues

Content

Korbeil commented 6 years ago

Any news ?

Korbeil commented 5 years ago

@Nyholm should we merge this ?

Korbeil commented 5 years ago

@Nyholm update :)

welcoMattic commented 5 years ago

May @bocharsky-bw or @Nyholm could review this 1 year old PR? :wink:

bocharsky-bw commented 5 years ago

Only @Nyholm can merge commits with failed TravisCI :/ We need to make tests green here first, then I'd be able to merge

UPD: Oh, wait, I even don't have access to this repo 😇

Korbeil commented 5 years ago

Only @Nyholm can merge commits with failed TravisCI :/ We need to make tests green here first, then I'd be able to merge

Tests are red cause HHVM does not support PHP anymore :) I already fixed it in https://github.com/php-translation/translator/pull/20

bocharsky-bw commented 5 years ago

I see #20 is merged, so could you rebase this PR with master? :)

Korbeil commented 5 years ago

@bocharsky-bw It's red on PHP 5.5, it's probably time to drop it ? What you think about it ?

See https://github.com/php-translation/translator/pull/21

welcoMattic commented 5 years ago

I think we can drop support of PHP 5.5 as its end of life was in 2016 ...

Korbeil commented 5 years ago

Just saw but, PHP 5.5 does not work since long time on Travis, see https://travis-ci.org/php-translation/translator/builds/548564414 (which was 1 month ago)

bocharsky-bw commented 5 years ago

Yes, let's drop 5.5 👍

Thank you for providing the PR. I think @Nyholm will be able to merge it soon

Simperfit commented 5 years ago

@Nyholm Do you have some time to review this please ? it seems that people will benefit from it ;)

Nyholm commented 5 years ago

I honestly not sure if it is beneficial or not. I've been reviewing this back and forth for a while.

It seams like the only thing we do is some refactoring and:

public function translateArray($strings, $from, $to)
    {
        $array = [];
         foreach ($strings as $string) {
            $array[] = $this->translate($string, $from, $to);
        }
         return $array;
    }

Why do we need that loop inside the implementation themselves?

Korbeil commented 5 years ago

@Nyholm this is only a bootstrap for https://github.com/php-translation/translator/issues/13 The idea is to send request with multiple strings to translate by default instead of translating one string by one. After that PR we have to implement this method for each provider but this translateArray will act as compatibility for all provider that don't support it already :)

Nyholm commented 5 years ago

Lets leave this PR for now. It contains too many refactoring and changes not related to the point you are trying to make.

Please create a new PR with a new interface that has the translateArray function and then pick one provider which implement that new interface.