Allow use with Pagerfanta 3.0 #298
Conversation
mbabker
force-pushed
the
feature/pagerfanta-3
branch
from
7478405
to
a51e1a2
Compare
|
If you wanted, sure. But from your perspective from this bundle since it’s
only using the query adapters, there aren’t really any API changes that
make supporting both versions complex (if anything the benefit is less CI
time). Considering the new major just shipped, and the breaking changes
are quite minimal (strictly typed API and minor template output changes)
I’d say it’s better to support both versions for a little while (even if
it’s just one or two minor versions) then go 3.0 only.
Also keep in mind that the UiBundle is still using deprecated APIs from
PagerfantaBundle. So 3.0 only becomes a blocker to allowing this bundle to
be upgraded in the main app until that issue is addressed.
On Tue, Mar 23, 2021 at 2:59 AM Łukasz Chruściel ***@***.***> wrote:
***@***.**** commented on this pull request.
Can't we just migrate to 3.0 only?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#298 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACZ7INXIDEZESL22TJVB3DTFBC5LANCNFSM4ZPAQLCA>
.
--
- Michael Please pardon any errors, this message was sent from my iPhone.
|
mbabker
force-pushed
the
feature/pagerfanta-3
branch
from
14f0726
to
4404859
Compare
|
I plan on EOL'ing the 2.x branch of the PagerfantaBundle at the end of this year, and the Pagerfanta library's 2.x branch won't get much support in 2022. So, moving forward with this and Sylius/SyliusGridBundle#143 would be really helpful in keeping Sylius from being locked to non-supported software branches (which also means Sylius/Sylius#12462 needs some focus to be able to fully complete the upgrade). |
|
Therefore, let's just migrate to 3.0 |
Done |
| @@ -76,7 +76,7 @@ function it_handles_Pagerfanta( | |||
|
|
|||
| $paginator->setMaxPerPage(5)->shouldBeCalled(); | |||
| $paginator->setCurrentPage(6)->shouldBeCalled(); | |||
| $paginator->getCurrentPageResults()->shouldBeCalled(); | |||
| $paginator->getCurrentPageResults()->willReturn([]); | |||
There was a problem hiding this comment.
Why did you change this line?
There was a problem hiding this comment.
When I originally did this the tests failed because of the lack of a return, most of the Pagerfanta package has return types declared in the 3.x branch.
|
Thanks, Michael! 🥇 |
|
It was a long run, but it finally landed in the codebase! :D 🎉 |
Pagerfanta and PagerfantaBundle both have stable 3.0 tags now, this PR adds support for the new major version.