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
Deprecate old auth tools #1689
Deprecate old auth tools #1689
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1689 +/- ##
==========================================
- Coverage 78.18% 78.09% -0.09%
==========================================
Files 106 106
Lines 13807 13847 +40
==========================================
+ Hits 10795 10814 +19
- Misses 3012 3033 +21 |
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.
Plz don't remove tests as this will conflict with #1683. We need to move tests to valid implementation.
Perhaps #1683 should disregard the deprecated modules?
Is that true? Or can we simply ignore them? If someone wishes to port/implement those tests for the new, preferred tools, I welcome that, but I'd consider that outside the scope of this effort. |
This pull request fixes 1 alert - view on lgtm.com fixed alerts:
Comment posted by lgtm.com |
3134dec
to
8e7c6c2
Compare
I've removed the commit that removed the tests. |
Actually, there's |
Hmm. I honestly don't understand how DeprecatedTool is meant to work. In the two places it's used, it's not wrapping another tool, but merely acting as a stand-in for another tool that was removed. I think we should remove the DeprecatedTool also, because its name betrays its purpose (it's not deprecating if it's replacing) and because there's no useful example of using it to deprecate a tool. |
This change is the first of two phases for #1688.