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

Produce deprecation warnings for deprecated names, remove unused imports #37

Merged
merged 3 commits into from
Nov 8, 2017

Conversation

jamadden
Copy link
Member

@jamadden jamadden commented Nov 8, 2017

In interfaces.py and registry.py and hookable.py

See #36 (comment)

Do this using zope.deferredimport and zope.deprecation, two new dependencies. This introduces a transitive dependency on zope.proxy, but that was already part of the 'security' extra. zope.proxy runs on pypy but it doesn't yet support making the C extension optional (zopefoundation/zope.proxy#26)

Also drop the use of _compat._BLANK everywhere and just use the literal.

Also remove unused but not BBB exported imports from _api.py in its own commit.

…d registry.py

And also in hookable.py

Do this using zope.deferredimport and zope.deprecation, two new
dependencies. This introduces a transitive dependency on zope.proxy,
but that was already part of the 'security' extra. zope.proxy runs
on pypy but it doesn't yet support making the C extension optional (zopefoundation/zope.proxy#26)

Also drop the use of _compat._BLANK everywhere and just use the literal.
Copy link
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

from zope.component._declaration import adapter
from zope.component._declaration import adapts
from zope.interface.interfaces import ComponentLookupError
from zope.interface.interfaces import IComponentLookup
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import ordering: stdlib (sys), 3rd-party (zope.hookable, zope.interface), then zope.component.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's twice in two days that I've been bitten by import groups in reviews. I'm beginning to appreciate Plone's policy of just ignoring that 😉

@jamadden
Copy link
Member Author

jamadden commented Nov 8, 2017

Thanks!

@jamadden jamadden merged commit 843d511 into master Nov 8, 2017
@jamadden jamadden deleted the remove-unused-imports branch November 8, 2017 17:06
@icemac
Copy link
Member

icemac commented Dec 14, 2017

@jamadden Maybe we could even work towards a 5.0 release which no longer contains these deprecation warnings as at least some of them are about 6 years old now.

@jamadden
Copy link
Member Author

@icemac Sure, we could absolutely eventually do that. I'm in no particular hurry though 😄 Now that the deprecation warnings are in place, the maintenance burden of them is pretty small (though I will admit there is a cognitive burden to those reading the code for the first time, a bit of technical debt).

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