-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix pre-prompt ccds version error #426
base: master
Are you sure you want to change the base?
Conversation
hooks/pre_prompt.py
Outdated
from ccds import __version__ | ||
|
||
if __name__ == "__main__": | ||
if version < "2.0.1": | ||
if __version__ < "2.0.1": |
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.
Is there any scenario where this would still break? You could potentially put this in a try-except.
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.
Yeah I think that's a good idea regardless.
…and removing any module-level imports from pre-prompt
4157c7c
to
131b123
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.
Change looks reasonable. How did you manually test different configs?
This fixes an issue introduced in #336 (hotfixed in #427 ) that added a pre-prompt hook to warn about the newly introduced versioning system.
However, the pre-prompt script was unable to access certain module-level imports from the ccds package.
(I'm still trying to figure out / think about why that's the case)I think this is because we simultaneously changed the versioning structure and mechanism in #336 by adding aversion.py
that computed the package version, and people with the old version ofccds
installed were looking for the version ofccds
in a module that didn't exist in their version of the package.Migrating version extraction from a separate
version.py
module into__init__.py
seems to fix this, since the version can be determined directly by importlib without needing separate imports from the ccds module.