Upgrade to Laravel 6, finally! #2243
Conversation
- Apparently, I forgot that `array_flatten` comes from Laravel. :) - When I did this previously, I did not search the views directory.
The Laravel changes with v6, and our translator is primarily an implementation of the Symfony contract.
It's contract will change in Laravel 6. We extend from Symfony's translator, but need to be compatible with that from Laravel in order to use its validation package. References: - https://laravel.com/docs/6.x/upgrade#trans-and-trans-choice - laravel/framework@8557dc5#diff-88bc04a1548d09aa6250d902d1ac2b4c
|
@tankerkiller125 offered to take care of replacing |
|
On the contrary, perhaps it would be better to merge this PR, break bundled extensions on dev/master, and that way, cleaning up bundled extensions would be easier? |
|
It's a pretty easy find-and-replace operation (and safer than relying on hitting all code paths with clicking through the app or relying on non-existent tests). |
|
@askvortsov1 I'll have all the core extensions updated this weekend anyway and I'll be committing directly to master, (no point in PRing a simple helper function to class function PR) so honestly a couple days at most wait to merge isn't that bad. |
| @@ -17,6 +17,16 @@ class Translator extends BaseTranslator implements TranslatorContract | |||
| { | |||
| const REFERENCE_REGEX = '/^=>\s*([a-z0-9_\-\.]+)$/i'; | |||
|
|
|||
| public function get($key, array $replace = [], $locale = null) | |||
There was a problem hiding this comment.
Can we do this the other way around, and have the logic in Laravel's new methods, and have the old methods reference the new ones?
There was a problem hiding this comment.
The "old" ones come from the Symfony base implementation.
There was a problem hiding this comment.
The reason I bring this up is that if we eventually want to comply with the laravel contract, we might as well move the logic to laravel's methods. If we want to eventually drop the laravel translation contract, this is fine though.
There was a problem hiding this comment.
I don't get what you mean, sorry. trans() and transChoice() are implemented in Symfony's translator, and we don't overwrite these. And since we're mostly/only using Symfony's interface throughout the app, I'd rather keep their implementation as well. These two methods are only needed as a bridge to be able to use this Symfony-based translator in Laravel's validation component.
|
Core extensions updated |
This seems to be a leftover change missed in #2243
Fixes #2055
This was easier than anticipated. 😄
Luckily, we are only using the translator using the interface from Symfony. We only need to fulfill Laravel's contract to be able to pass something into its validator factory.
Changes proposed in this pull request:
Translatorcontract (from Laravel) in our implementation - only needed for the validation componentConfirmed
Frontend changes: tested on a local Flarum installation.composer test).