Skip to content

Update LaravelLocalization.php #407

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

Closed
wants to merge 1 commit into from

Conversation

lukasfiala
Copy link

No description provided.

@mcamara
Copy link
Owner

mcamara commented Feb 17, 2017

Can you provide more information about what your pr fixes? And don't forget to fix the tests!

Thanks!

@mcamara
Copy link
Owner

mcamara commented Feb 18, 2017

If you rebase with master, you will fix the errors in your branch.

@Okipa
Copy link

Okipa commented Mar 3, 2017

Any news on this guys ?

@bmichotte bmichotte mentioned this pull request Mar 8, 2017
@Okipa
Copy link

Okipa commented Apr 10, 2017

Shouldn't this PR change the line 325 from the LaravelLocalization.php from $this->translator->trans($transKeyName, [], $locale) to $this->translator->trans($transKeyName, [], null, $locale) rather than $this->translator->trans($transKeyName, [], '', $locale) ?

(Same idea for the line 635).

@mcamara
Copy link
Owner

mcamara commented Apr 10, 2017

Hi @Okipa , can you lend me a hand on this please? If you create a PR fixing it I'll merge it into master and create a new version.

Thanks!

@Okipa
Copy link

Okipa commented Apr 10, 2017

After investigations, I think I found the problem, which is not linked to your code but to the readme that is not accurate about which version of the package use with which version of Laravel.
I didn't find the logic but what is sure is that I had to install the version 1.1.* of your package with Laravel 5.2 to make the routes translations work and not the 1.0.* one as it is specified (it does not work with the 1.2.*either).

@mcamara
Copy link
Owner

mcamara commented Apr 25, 2017

Closing this PR as it was fixed in the new docs.

@mcamara mcamara closed this Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants