Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen added bug 3. to review Waiting for reviews feature: language/translations (l10n/i18n) Localization and translation matters labels Mar 30, 2021
@nickvergessen nickvergessen added this to the Nextcloud 22 milestone Mar 30, 2021
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@nickvergessen nickvergessen force-pushed the techdebt/noid/symfony-component-translation-pluralization-rules-is-deprecated branch from bede060 to 76f7b70 Compare March 30, 2021 19:07
@rullzer
Copy link
Member

rullzer commented Mar 31, 2021

CI does not agree

@nickvergessen
Copy link
Member Author

Fun, showed a broken string missing plurals in the test, but only with this I noticed that our placeholders dont work anymore

@nickvergessen
Copy link
Member Author

Fixed by readding vsprintf at the end and adding a test in the L10nTest for it

@MorrisJobke
Copy link
Member

CI is still very unhappy 😢


// No exclude groups
$data[] = ['no', null, null, null, false];
$data[] = ['no', null, null, [], false];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Test\Share20\ManagerTest::testIsSharingDisabledForUser with data set #0 ('no', null, null, null, false)
    Method getUserGroupIds may not return value of type NULL, its return declaration is ": array"

@nickvergessen nickvergessen force-pushed the techdebt/noid/symfony-component-translation-pluralization-rules-is-deprecated branch from bb415e4 to 79ebc7f Compare April 20, 2021 14:51
@nickvergessen
Copy link
Member Author

@MorrisJobke good to go?

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine 👍

@MorrisJobke MorrisJobke merged commit 012f791 into master Apr 22, 2021
@MorrisJobke MorrisJobke deleted the techdebt/noid/symfony-component-translation-pluralization-rules-is-deprecated branch April 22, 2021 19:20
@MorrisJobke
Copy link
Member

@nickvergessen Should this be backported? Was requested in 012f791#commitcomment-49915825

@nickvergessen
Copy link
Member Author

So it's deprecated, should only log in debug? or does it log normally?

@ghost
Copy link

ghost commented Apr 24, 2021

@nickvergessen It logs on several occasions as error

**Error**: Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as the first argument is deprecated since Symfony 4.3, pass it as the second argument and provide the event object as the first argument instead. at /var/www/nextcloud/3rdparty/symfony/event-dispatcher/EventDispatcher.php#58

1. *<<closure>>*
   OC\Log\ErrorHandler::onError()

2. */var/www/nextcloud/3rdparty/symfony/event-dispatcher/EventDispatcher.php* *- line 58:*
   trigger_error()

3. */var/www/nextcloud/apps/issuetemplate/lib/Controller/APIController.php* *- line 68:*
   Symfony\Component\EventDispatcher\EventDispatcher->dispatch()

4. */var/www/nextcloud/apps/issuetemplate/lib/Controller/APIController.php* *- line 84:*
   OCA\IssueTemplate\Controller\APIController->queryAppDetails()

5. */var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php* *- line 218:*
   OCA\IssueTemplate\Controller\APIController->details()

6. */var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php* *- line 127:*
   OC\AppFramework\Http\Dispatcher->executeController()

7. */var/www/nextcloud/lib/private/AppFramework/App.php* *- line 157:*
   OC\AppFramework\Http\Dispatcher->dispatch()

8. */var/www/nextcloud/lib/private/Route/Router.php* *- line 302:*
   OC\AppFramework\App::main()

9. */var/www/nextcloud/lib/base.php* *- line 993:*
   OC\Route\Router->match()

10. */var/www/nextcloud/index.php* *- line 37:*
    OC::handleRequest()

or this one

**Error**: The "Symfony\Component\Translation\PluralizationRules" class is deprecated since Symfony 4.2. at /var/www/nextcloud/3rdparty/symfony/translation/PluralizationRules.php#38

1. *<<closure>>*
   OC\Log\ErrorHandler::onError()

2. */var/www/nextcloud/3rdparty/symfony/translation/PluralizationRules.php* *- line 38:*
   trigger_error()

3. */var/www/nextcloud/lib/private/L10N/L10N.php* *- line 226:*
   Symfony\Component\Translation\PluralizationRules::get()

4. */var/www/nextcloud/lib/private/L10N/L10NString.php* *- line 70:*
   OC\L10N\L10N->OC\L10N\{closure}(**"\**\* sensiti ... \*"**)

5. */var/www/nextcloud/lib/private/L10N/L10N.php* *- line 131:*
   OC\L10N\L10NString->__toString()

6. */var/www/nextcloud/lib/private/L10N/LazyL10N.php* *- line 57:*
   OC\L10N\L10N->n()

7. */var/www/nextcloud/lib/private/DateTimeFormatter.php* *- line 257:*
   OC\L10N\LazyL10N->n()

8. */var/www/nextcloud/apps/settings/lib/Controller/CheckSetupController.php* *- line 534:*
   OC\DateTimeFormatter->formatTimeSpan()

9. */var/www/nextcloud/apps/settings/lib/Controller/CheckSetupController.php* *- line 732:*
   OCA\Settings\Controller\CheckSetupController->getLastCronInfo()

10. */var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php* *- line 218:*
    OCA\Settings\Controller\CheckSetupController->check()

11. */var/www/nextcloud/lib/private/AppFramework/Http/Dispatcher.php* *- line 127:*
    OC\AppFramework\Http\Dispatcher->executeController()

12. */var/www/nextcloud/lib/private/AppFramework/App.php* *- line 157:*
    OC\AppFramework\Http\Dispatcher->dispatch()

13. */var/www/nextcloud/lib/private/Route/Router.php* *- line 302:*
    OC\AppFramework\App::main()

14. */var/www/nextcloud/lib/base.php* *- line 993:*
    OC\Route\Router->match()

15. */var/www/nextcloud/index.php* *- line 37:*
    OC::handleRequest()

@nickvergessen
Copy link
Member Author

But if you get this in your log on a production instance it seems you have debug enabled, which you should disable, can you confirm?

@nickvergessen
Copy link
Member Author

Nevermind, I found the problem... feel free to test the branch at #26762

@ghost
Copy link

ghost commented Apr 26, 2021

Nevermind, I found the problem... feel free to test the branch at #26762

I was allready doubting myself. 🥴😄 Glad you found it.🥳

@ghost
Copy link

ghost commented Apr 26, 2021

Works. 🛸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug feature: language/translations (l10n/i18n) Localization and translation matters

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants