-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support mutiple strings in same request #14
Support mutiple strings in same request #14
Conversation
488124a
to
466e4de
Compare
Any news ? |
@Nyholm should we merge this ? |
@Nyholm update :) |
May @bocharsky-bw or @Nyholm could review this 1 year old PR? 😉 |
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 😇 |
I see #20 is merged, so could you rebase this PR with master? :) |
67b5404
to
67ae8b1
Compare
@bocharsky-bw It's red on PHP 5.5, it's probably time to drop it ? What you think about it ? See #21 |
I think we can drop support of PHP 5.5 as its end of life was in 2016 ... |
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) |
Yes, let's drop 5.5 👍 Thank you for providing the PR. I think @Nyholm will be able to merge it soon |
@Nyholm Do you have some time to review this please ? it seems that people will benefit from it ;) |
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? |
@Nyholm this is only a bootstrap for #13 |
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 |
Issues
Content
TranslatorService
interface withtranslateArray(array $strings, string $from, string $to): array
method.AbstractTranslator
to handle default behavior fortranslateArray()
(which is just callingtranslate()
method for all strings).translateArray()
method & updatingtranslate()
method.format()
method toAbstractTranslator
& making it fixing translated string with first letter uppercase when original isn't.