Skip to content

Support pagy v43#140

Merged
davidcelis merged 1 commit intodavidcelis:masterfrom
buildrtech:support-pagy-v43
Nov 20, 2025
Merged

Support pagy v43#140
davidcelis merged 1 commit intodavidcelis:masterfrom
buildrtech:support-pagy-v43

Conversation

@mcasper
Copy link
Copy Markdown
Contributor

@mcasper mcasper commented Nov 4, 2025

Pagy recently released v43, which contained some breaking API changes that affect this gem as well: https://ddnexus.github.io/pagy-pre/guides/upgrade-guide/. This PR updates the gem to detect when it's being used with Pagy >= v43, and to use the appropriate new APIs to interact with Pagy

Unrelated to the Pagy bump, I had to add several dev dependencies to get tests running on my machine on Ruby 3.4.x. I tried downgrading to one of the Ruby versions listed in the travis config, but they all failed to install on my machine. Happy to remove this bit of the code if it's not wanted!

@slhck
Copy link
Copy Markdown
Contributor

slhck commented Nov 7, 2025

Thanks for this! Maintenance of this gem seems a bit… dormant. Let's hope this gets a new release soon 🤞

@davidcelis
Copy link
Copy Markdown
Owner

davidcelis commented Nov 7, 2025

Thanks for the PR, @mcasper; I haven't used this gem myself for a long time at this point (hence the, erm, "dormant" maintenance). I'm happy to push a new release to support pagy v43, but I don't think it needs to be backwards compatible. Let's just drop support for the older version. Hopefully I can find some time to get the CI setup working on GitHub Actions (I don't use Travis anymore) so that I can release a new version of this gem. At the very least, getting something merged in would let people point their Gemfile at this repo.

@slhck
Copy link
Copy Markdown
Contributor

slhck commented Nov 7, 2025

I created #141 to test whether GHA works, replacing Travis.

@slhck
Copy link
Copy Markdown
Contributor

slhck commented Nov 8, 2025

@mcasper Now tests via GitHub Actions are in place – I also had to slightly change the Gemfile to properly limit the pagy dependencies to not install v43 just yet. If you could please rebase from master and reapply your changes?

@mcasper mcasper force-pushed the support-pagy-v43 branch 2 times, most recently from 83412fe to 1bd47d0 Compare November 10, 2025 07:02
@mcasper
Copy link
Copy Markdown
Contributor Author

mcasper commented Nov 10, 2025

Thank you @slhck for getting the tests running! I've rebased this against master.

@davidcelis I also went ahead and dropped support for older versions of Pagy like you asked, which made the diff nice and simple 😄 let me know what else I can do!

@hoshy hoshy mentioned this pull request Nov 13, 2025
@davidcelis
Copy link
Copy Markdown
Owner

@mcasper I overhauled the GHA workflows a little bit; would you please rebase off of the main branch again to pick up those changes? Hopefully you don't hit bad conflicts 😅

@slhck
Copy link
Copy Markdown
Contributor

slhck commented Nov 14, 2025

Sorry for chiming in on this — it's your call of course — but considering the different issues that I found with older Ruby versions, wouldn't it make sense to keep testing support for 3.3 at least? (3.2 has gone out of maintenance but is still not EOL until March of 2026.)

@davidcelis
Copy link
Copy Markdown
Owner

The issues were only related to test suite setup, no? Or were there issues in the actual library code?

@slhck
Copy link
Copy Markdown
Contributor

slhck commented Nov 14, 2025

Yes, it was the test setup only.

@davidcelis
Copy link
Copy Markdown
Owner

I feel comfortable not having the Ruby test version matrix, then; I want to try to find some time to just make a simple PORO instead of having that janky Integer monkeypatch stick around. I had forgotten I'd done that and was embarrassed to see it 😂

Pagy v43 contains some major changes to the API, primarily moving all
options into Pagy.options, and forcing you to choose a paginator instead
of directly using the Pagy class. This commit requires the use of Pagy >= v43
going forward, and updates the integration to use the new API.
@mcasper
Copy link
Copy Markdown
Contributor Author

mcasper commented Nov 20, 2025

Thanks for fixing up CI @davidcelis! I've rebased back over the main branch and squashed everything into a single commit 👍

@davidcelis
Copy link
Copy Markdown
Owner

Thanks for the group effort, everyone!

@davidcelis davidcelis merged commit 55fc4a0 into davidcelis:master Nov 20, 2025
4 checks passed
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