-
Notifications
You must be signed in to change notification settings - Fork 58
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
remove deprecated tests.helpers #1597
Conversation
asdf/tests/helpers.py
Outdated
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.
There is a lingering reference to this file in conftest.py:
https://github.com/asdf-format/asdf/blob/main/asdf/conftest.py#L4
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.
Also a comment in asdf._tests._helpers that needs an update:
https://github.com/asdf-format/asdf/blob/main/asdf/_tests/_helpers.py#L421
Maybe now that the entire module is underscored we can just import those modules at the top?
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.
Aaaaand one of the tox.ini environments ignores the deprecation warning from asdf.tests.helpers:
https://github.com/asdf-format/asdf/blob/main/tox.ini#L146
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.
Thanks! I updated the conftest, comment and tox.ini in 6a4027a.
I also moved the imports to the top. Hopefully those imports and much of _tests/_helpers
can be deleted once the legacy extension system is gone.
@@ -81,5 +81,5 @@ include adding block storage support to :ref:`converters <extending_converters>` | |||
asdf.tests.helpers Deprecation | |||
============================== | |||
|
|||
Use of `asdf.tests.helpers` is deprecated. Please see `asdf.testing.helpers` | |||
Use of ``asdf.tests.helpers`` is deprecated. Please see `asdf.testing.helpers` |
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.
Now that the module has been removed, do we remove its deprecations section? Or change the language to state that it has been "removed"?
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.
I was thinking we'd leave this for at least 3.X as users migrating from <=2.15 might benefit from not needing to open older docs (2.15) to see what was deprecated/removed. It might end up being redundant if we can put together an exhaustive (but not exhausting) release doc for 3.0.
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.
LGTM
6a4027a
to
185fdbb
Compare
No description provided.