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

Add an example for patching methods with pagination #11

Closed
wants to merge 1 commit into from
Closed

Add an example for patching methods with pagination #11

wants to merge 1 commit into from

Conversation

aidistan
Copy link
Contributor

This PR proposes an example for patching methods with pagination.

And it would be better to support paginations natively.

@mgmarlow
Copy link
Owner

Hello again! Thanks for taking the time to write this up. I'm not sure if I agree on including this example with the project, as I don't want to recommend that users of the gem monkey-patch modules that are considered part of the gem's private API. In other words, I don't want to cause bugs by changing the names or namespaces of these modules :)

@aidistan
Copy link
Contributor Author

I have done researches on ruby gems for Notion APIs and found this gem my favorite. However for my own experience, supporting cursor pagination is a must-have feature which has been implemented in other competitors such as notion-ruby-client.

Thus I wrote these patches in a DRY way for my own project alongside others irrelated to this gem such as Notion::Block#to_md 😄

This PR is just a proposal to supporting the feature, either by providing an example, or, in a preferable way, implementing methods with pagniations natively in the gem.

@mgmarlow mgmarlow mentioned this pull request Aug 24, 2022
@mgmarlow
Copy link
Owner

Agreed, it would be nice to have the SDK assist with pagination, at least when it comes down to handling retries and respectfully querying the Notion APIs.

This is definitely something the SDK should implement, so I've opened up #12 to address it.

@aidistan
Copy link
Contributor Author

Glad the idea proposed here has been addressed in a far more delicate way (#12). Hope it get closed soon.

I see no point to keep this PR, thus decide to close it.

@aidistan aidistan closed this Aug 26, 2022
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.

2 participants