-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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 base class for api/v1/trends/*
controllers
#27841
Conversation
e5b29cf
to
a19c096
Compare
508f1dc
to
1185fb7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #27841 +/- ##
==========================================
- Coverage 85.01% 85.01% -0.01%
==========================================
Files 1059 1061 +2
Lines 28277 28308 +31
Branches 4538 4537 -1
==========================================
+ Hits 24040 24065 +25
- Misses 3074 3078 +4
- Partials 1163 1165 +2 ☔ View full report in Codecov by Sentry. |
1185fb7
to
05520a0
Compare
05520a0
to
0832b61
Compare
0832b61
to
ce1ec74
Compare
Much of the base class extraction here accomplishes some of what the 3 other recent api/pagination-related PRs do. Will rebase this after those are in and see what's left. |
This pull request has merge conflicts that must be resolved before it can be merged. |
ce1ec74
to
834c3e1
Compare
This pull request has resolved merge conflicts and is ready for review. |
834c3e1
to
fcd6740
Compare
fcd6740
to
a8fe66d
Compare
Closing this in favor of the few remaining api pagination PRs that are open (which do almost all of this themselves). |
Similar to #27797 for
api/v1/instances
and #27794 forapi/v1/statuses
this pulls out some common setup within theapi/v1/trends/*
controllers.Most of the shared behaviour here is around pagination of the api response. I think there might be opportunity here for further extraction of a shared controller concern for pagination across all the API controllers, but starting small here with a few base class extractions.
With #27837 merged, we are at 100% coverage for these. With changes, coverage stays at 100% across controllers, behavior should not change.