Allow Symfony 5

[emodric]

Let's see if this works :)

[emodric]

Hm.. No Travis :/

[rpkamp]

There is Travis, but for some reason it's not showing up in this MR.
It is failing though, see https://travis-ci.org/whiteoctober/WhiteOctoberPagerfantaBundle/builds/615494595

[alex-bacart]

Probably the Travis config https://github.com/whiteoctober/WhiteOctoberPagerfantaBundle/blob/master/.travis.yml should be also changed.

[alex-bacart]

I suppose this line should match the PHP version from Symfony.
https://github.com/symfony/symfony/blob/5.0/composer.json#L19
Then Travis will pass.. maybe :)

[emodric]

Will try to work on it next week, as soon as I'm back at work.

This was just a quick test to see if it would work, directly from SymfonyCon :)

[mynameisbogdan]

It's okay at is. It should, but not necessarily, match the PHP version from symfony if this bundle supports only symfony 5.0.

[sampart]

Thanks so much for your involvement, @emodric. As this project isn't actively maintained, you may be interested in setting up a fork (as described in that link). That said, I should be able to merge this change once it's ready. Thanks again.

[emodric]

Hi @sampart

Currently, I have no plans to fork the library, maybe in the future. Even if the repo is not actively maintained, I still think there's value in it being here still, as most of the community defaults to looking here for it.

Anyways,

All Travis failures are related to using obsolete versions of PHP which simply timeout on Travis and never install the dependencies (probably due to large dependency graphs). How would you feel about creating a new major version which only supports Symfony ^4.4 || ^5.0 and only PHP >= 7.2.5 ?

[sampart]

Does this branch as it stands work correctly with Symfony 5? If so, I'm happy to merge it so that people who are using a newer Symfony version can use this, and to then resolve the Travis problems separately (if at all).

Thanks for your persistence and involvement here, and sorry not to be able to be more actively involved myself - I have permissions on this repo and don't want to leave its users in the lurch, but I'm not in a position to maintain it myself, hence the call for maintainers to fork it.

[mbabker]

Does this branch as it stands work correctly with Symfony 5?

As this PR is it's impossible to clone the repo, apply the changes from this pull request, and install everything with Symfony 5.x.

A couple of notes on needed updates:

• symfony/symfony shouldn't be a dev dependency, explicitly require each component needed for dev or production
• PHPUnit needs updating; the max allowed version is PHPUnit 5.7 which requires symfony/yaml and doesn't support Symfony 5

I currently have this diff applied locally which is the changes from this PR and PHPUnit related changes to get everything to install at Symfony 5.x and let PHPUnit run on my PHP 7.3 localhost:

diff --git a/Tests/View/TranslatedViewTest.php b/Tests/View/TranslatedViewTest.php
index a3a1c28..41ef210 100644
--- a/Tests/View/TranslatedViewTest.php
+++ b/Tests/View/TranslatedViewTest.php
@@ -11,7 +11,9 @@
 
 namespace WhiteOctober\PagerfantaBundle\Tests\View;
 
-abstract class TranslatedViewTest extends \PHPUnit_Framework_TestCase
+use PHPUnit\Framework\TestCase;
+
+abstract class TranslatedViewTest extends TestCase
 {
     private $view;
     private $translator;
@@ -21,7 +23,7 @@ abstract class TranslatedViewTest extends \PHPUnit_Framework_TestCase
     private $pagerfanta;
     private $routeGenerator;
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $this->view = $this->createViewMock();
         $this->translator = $this->createTranslatorMock();
@@ -188,4 +190,4 @@ abstract class TranslatedViewTest extends \PHPUnit_Framework_TestCase
     }
 
     abstract protected function translatedViewName();
-}
\ No newline at end of file
+}
diff --git a/composer.json b/composer.json
index c37e870..853a68b 100644
--- a/composer.json
+++ b/composer.json
@@ -13,14 +13,14 @@
     "require": {
         "php": ">=5.3.3",
         "pagerfanta/pagerfanta": "^1.1.0|^2.0.0",
-        "symfony/framework-bundle": "~2.3|~3.0|~4.0",
-        "symfony/property-access": "~2.3|~3.0|~4.0",
-        "symfony/translation": "~2.3|~3.0|~4.0",
-        "symfony/twig-bundle": "~2.3|~3.0|~4.0"
+        "symfony/framework-bundle": "~2.3|~3.0|~4.0|~5.0",
+        "symfony/property-access": "~2.3|~3.0|~4.0|~5.0",
+        "symfony/translation": "~2.3|~3.0|~4.0|~5.0",
+        "symfony/twig-bundle": "~2.3|~3.0|~4.0|~5.0"
     },
     "require-dev": {
-        "symfony/symfony": "~2.3|~3.0|~4.0",
-        "phpunit/phpunit": "~3.7|~4.0|^5.0"
+        "symfony/symfony": "~2.3|~3.0|~4.0|~5.0",
+        "phpunit/phpunit": "~3.7|~4.0|^5.0|~6.0|~7.0|~8.0"
     },
     "conflict": {
         "twig/twig": "<1.34|>=2.0,<2.4"

Unfortunately, while PHPUnit runs, the tests aren't actually testing anything because of a reliance on the deprecated Symfony\Component\Translation\TranslatorInterface:

Michaels-Mac-mini:WhiteOctoberPagerfantaBundle mbabker$ vendor/bin/phpunit 
PHPUnit 8.5.0 by Sebastian Bergmann and contributors.

WWWWWWWWWWWWWWWWWWWWWWWW                                          24 / 24 (100%)

Time: 110 ms, Memory: 6.00 MB

There were 24 warnings:

1) WhiteOctober\PagerfantaBundle\Tests\View\DefaultTranslatedViewTest::testRenderShouldTranslatePreviuosAndNextMessage
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

2) WhiteOctober\PagerfantaBundle\Tests\View\DefaultTranslatedViewTest::testRenderAllowsCustomizingPreviousMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

3) WhiteOctober\PagerfantaBundle\Tests\View\DefaultTranslatedViewTest::testRenderAllowsCustomizingNextMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

4) WhiteOctober\PagerfantaBundle\Tests\View\DefaultTranslatedViewTest::testGetNameShouldReturnTheName
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

5) WhiteOctober\PagerfantaBundle\Tests\View\SemanticUiTranslatedViewTest::testRenderShouldTranslatePreviuosAndNextMessage
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

6) WhiteOctober\PagerfantaBundle\Tests\View\SemanticUiTranslatedViewTest::testRenderAllowsCustomizingPreviousMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

7) WhiteOctober\PagerfantaBundle\Tests\View\SemanticUiTranslatedViewTest::testRenderAllowsCustomizingNextMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

8) WhiteOctober\PagerfantaBundle\Tests\View\SemanticUiTranslatedViewTest::testGetNameShouldReturnTheName
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

9) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testRenderShouldTranslatePreviuosAndNextMessage
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

10) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testRenderAllowsCustomizingPreviousMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

11) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testRenderAllowsCustomizingNextMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

12) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testGetNameShouldReturnTheName
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

13) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap3TranslatedView::testRenderShouldTranslatePreviuosAndNextMessage
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

14) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap3TranslatedView::testRenderAllowsCustomizingPreviousMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

15) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap3TranslatedView::testRenderAllowsCustomizingNextMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

16) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap3TranslatedView::testGetNameShouldReturnTheName
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

17) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap4TranslatedView::testRenderShouldTranslatePreviuosAndNextMessage
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

18) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap4TranslatedView::testRenderAllowsCustomizingPreviousMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

19) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap4TranslatedView::testRenderAllowsCustomizingNextMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

20) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrap4TranslatedView::testGetNameShouldReturnTheName
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

21) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testRenderShouldTranslatePreviuosAndNextMessage
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

22) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testRenderAllowsCustomizingPreviousMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

23) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testRenderAllowsCustomizingNextMessageWithOption
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

24) WhiteOctober\PagerfantaBundle\Tests\View\TwitterBootstrapTranslatedViewTest::testGetNameShouldReturnTheName
Cannot stub or mock class or interface "Symfony\Component\Translation\TranslatorInterface" which does not exist

WARNINGS!
Tests: 24, Assertions: 0, Warnings: 24.

So, it's going to take a bit more effort to make this bundle properly compatible with Symfony 5.

[sampart]

Thanks for taking the time to look into this, @mbabker. Your patch will definitely be useful to the community.

As alluded to earlier, there's no-one at White October who is able to maintain this code, and although I have write permissions on the repo, I'm also not in a position to develop it or test changes - my access is so that I can help someone else take this on.

Given that, and the scope of the changes required for Symfony 5, I think we would be reliant on someone else to fork this repo and make these changes there.

As discussed on the "looking for maintainers" issue, such a person wouldn't be committing to maintaining this from the start. Rather, once the fork had advanced sufficiently that they and we were happy that it was viable, we could link to it from here. There'd be nothing to stop others using it prior to that, of course, so anyone who needs Symfony 5 support now should of course feel free to make a fork and apply the changes, whether or not they plan on doing further development in future. That probably goes without saying, but I thought it worth being explicit!

Furthermore, as has happened with https://github.com/whiteoctober/BreadcrumbsBundle, the fork need only be for newer versions of Symfony (>=4.4 in this case). That would make ongoing maintenance easier.

Anyway, here's hoping that someone is able to move this forward. I've updated the "looking for maintainers" issue to make the point about only supporting newer versions being an option.

Thanks again for everyone's input so far.

[mbabker]

OK, since the bundle here is actually pretty low maintenance on it's own, I went ahead and took a few hours to clean things up into a forked version, now available at https://github.com/BabDev/BabDevPagerfantaBundle. Aside from a namespace change it should be a quick drop-in replacement.

I don't have the bandwidth to dig into the main Pagerfanta library and put time in there, but for the sake of some of my own work and the people relying on this bundle as the glue to get Pagerfanta integrated nicely into their Symfony apps, I can look after the bundle for the foreseeable future.

[sampart]

That's brilliant, @mbabker, thank you very much indeed! 💙 Are you happy for me to link to your fork from the README of this one?

[mbabker]

Go for it!

[sampart]

Excellent, done! Thanks again @mbabker, and thanks to @emodric for kicking off this discussion in the first place.

[emodric]

Sorry I couldn't be of much help further :(

[mbabker]

No need to be sorry @emodric, if anything it helped point out the B/C breaks that needed to be accounted for by just changing the version constraints.

[olorton]

@sampart Should we do something about Packagist? https://packagist.org/packages/white-october/pagerfanta-bundle

[sampart]

Hi @olorton 👋. What sort of thing did you have in mind?

[olorton]

Hello @sampart 👋

I think that no further action is required, it took a while for packagist to update the not actively maintained, here's the fork message. I'm happy with this now.

[sampart]

Ah cool, thanks for checking.