Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace cursor_pagination with pagy_cursor #158

Merged

Conversation

crazyoptimist
Copy link
Collaborator

@crazyoptimist crazyoptimist commented Jan 5, 2023

Close #148 #126
cursor_pagination is abandoned and no longer maintained.
This PR replaces it with pagy_cursor, a cursor pagination gem based on pagy.

@crazyoptimist crazyoptimist force-pushed the chore/replace-cursor-with-pagy branch from d32b98a to 8ea45ad Compare January 5, 2023 09:30
@crazyoptimist
Copy link
Collaborator Author

@dblock this is ready for review.

@dblock
Copy link
Collaborator

dblock commented Jan 5, 2023

Nice! Since this is a breaking change, let's make the version 2.0 in this PR (CHANGELOG and version.rb please).
We also mention cursor_pagination in README and possibly elsewhere.

@dangerpr-bot
Copy link

1 Warning
⚠️ There're library changes, but not tests. That's OK as long as you're refactoring existing code.

Generated by 🚫 Danger

@crazyoptimist
Copy link
Collaborator Author

Done. Seems like tests fail due to the rate limit, which is fine.

@dblock dblock merged commit d884403 into slack-ruby:master Jan 6, 2023
@dblock
Copy link
Collaborator

dblock commented Jan 6, 2023

Merged, thanks!

@dblock
Copy link
Collaborator

dblock commented Jan 6, 2023

@crazyoptimist
Copy link
Collaborator Author

Could you manually run the failed workflows again? I guess it's github.

@crazyoptimist
Copy link
Collaborator Author

I synced my master with the upstream, and ran the tests. Everything is green.
image

@crazyoptimist
Copy link
Collaborator Author

crazyoptimist commented Jan 6, 2023

This might be related. Probably smaller runner image is assigned or something else during the workflow run. 9 of 10, not an issue in the code.

@dblock
Copy link
Collaborator

dblock commented Jan 8, 2023

@crazyoptimist
Copy link
Collaborator Author

Yes, it's github. So running the workflow again manually will just be fine, like when GHA servers get less load.

@dblock
Copy link
Collaborator

dblock commented Jan 8, 2023

Looks like this is angular/webdriver-manager#216. We need to have reliable CI, looking at this.

@dblock
Copy link
Collaborator

dblock commented Jan 8, 2023

@crazyoptimist Care to add a section to UPGRADING.md? I believe at the very least users will need to swap gems in their Gemfile.

@dblock
Copy link
Collaborator

dblock commented Jan 8, 2023

Yes, it's github. So running the workflow again manually will just be fine, like when GHA servers get less load.

#159

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.

Replace cursor_pagination
3 participants