-
Notifications
You must be signed in to change notification settings - Fork 69
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
feat(taps): Implement reference paginators #732
feat(taps): Implement reference paginators #732
Conversation
b010229
to
5a184a4
Compare
3180eea
to
9952838
Compare
9952838
to
029410d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edgarrmondragon - This is so cool!
My comments are only cosmetic, more about getting this ready for a prod release.
I think it's worth discussing if we should warn (as you have it) or just fail if they are using the legacy approach. And if we fail, would be nice to do so in a unit test (nested in get_standard_tests or similar) rather than at runtime. 😄
Co-authored-by: Aaron ("AJ") Steers <aj@meltano.com>
Codecov Report
@@ Coverage Diff @@
## main #732 +/- ##
==========================================
+ Coverage 80.68% 81.27% +0.58%
==========================================
Files 35 36 +1
Lines 3480 3520 +40
Branches 692 706 +14
==========================================
+ Hits 2808 2861 +53
+ Misses 499 486 -13
Partials 173 173
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @edgarrmondragon !
Just one small question (nit).
28a98fb
to
8044212
Compare
@aaronsteers I think we could warn that it will fail in the future. Wdyt? |
Docs: https://meltano-sdk--732.org.readthedocs.build/en/732
Closes #316