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

[Feature]: pagination for source http #2847

Merged
merged 22 commits into from
Jul 25, 2023
Merged

[Feature]: pagination for source http #2847

merged 22 commits into from
Jul 25, 2023

Conversation

davama
Copy link
Contributor

@davama davama commented Jun 29, 2023

Pre-Request Checklist

  • Passes rubocop code analysis (try rubocop --auto-correct)
  • Tests added or adapted (try rake test)
  • Changes are reflected in the documentation
  • User-visible changes appended to CHANGELOG.md

Description

The burden of getting ALL data should not fall on the api but on the client.

We have several applications that use pagination and thought it was time for oxidized to have it too.

Basically adds two new settings to source.http.
See documentation for details.

Does not break existing config settings who do not have these settings enabled.

Thanks

EDIT

forgot to mention that I am not a ruby dev, just a IT enthusiast. So any ruby code syntax suggestions are welcomed

@davama
Copy link
Contributor Author

davama commented Jun 29, 2023

Will try to fix the CI checks

@davama
Copy link
Contributor Author

davama commented Jun 30, 2023

ok
i've done all the code related changes.

not sure what to make of the codecov/patch and codecov/project

as far as the module, it works as described in the first comment

Please let me know what I can do to make the checks pass.

Thanks

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2023

Codecov Report

Patch coverage: 8.00% and project coverage change: -0.66 ⚠️

Comparison is base (91994e9) 66.55% compared to head (4c02357) 65.90%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2847      +/-   ##
==========================================
- Coverage   66.55%   65.90%   -0.66%     
==========================================
  Files          30       30              
  Lines        1507     1525      +18     
==========================================
+ Hits         1003     1005       +2     
- Misses        504      520      +16     
Impacted Files Coverage Δ
lib/oxidized/source/http.rb 31.88% <8.00%> (-7.34%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@davama
Copy link
Contributor Author

davama commented Jul 10, 2023

all checks passed! this is great!!

@davama
Copy link
Contributor Author

davama commented Jul 12, 2023

This is ready to be merged 👍

@mortzu mortzu merged commit 82e434b into ytti:master Jul 25, 2023
5 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