-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Wrong version in __init__ #55
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
Conversation
python/lookout/__init__.py
Outdated
@@ -2,4 +2,3 @@ | |||
# DO NOT CHANGE OR ADD ANYTHING HERE |
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 think the changes need to be done in https://github.com/src-d/lookout-sdk/blob/master/_tools/protogen_python.sh#L23
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.
ohhhh. thanks!
I don't think we should change anything besides generated code from proto when we run protogen.
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.
removed here: 7ba6124
@@ -0,0 +1 @@ | |||
__version__ = "0.2.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.
It is more common to specify this as a tuple:
__version__ = 0, 2, 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.
I also thought that but apparently, it's not true.
Here is example of 4 different VERY common libs all of them use string version:
https://github.com/urllib3/urllib3/blob/1.24.1/src/urllib3/__init__.py#L30
https://github.com/requests/requests/blob/master/requests/__version__.py#L8
https://github.com/chardet/chardet/blob/master/chardet/version.py#L8
https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/__init__.py#L133
Though we can add VERSION
variable also as chardet
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.
LOL you are right https://www.python.org/dev/peps/pep-0396
A good example of poor design... I have to live with it
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.
For reference, another example of string version can be found in the docs here: https://packaging.python.org/tutorials/packaging-projects/#creating-setup-py
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.
Wow! The PEP is from 2011! But I have never heard of it!
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.
Oh wait, this change needs to be reflected here: https://github.com/src-d/lookout-sdk/blob/master/docs/CONTRIBUTING.md#release-process
🤦♂️ I learn something new about this repository every day. Thanks, fixed! |
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.
👍
Use only one source of truth for the version same as any other major library does. Signed-off-by: Maxim Sukharev <max@smacker.ru>
Signed-off-by: Maxim Sukharev <max@smacker.ru>
Signed-off-by: Maxim Sukharev <max@smacker.ru>
Use only one source of truth for the version same as any other major
library does.
As examples of such libs: urllib3, requests, chardet, SQLAlchemy.
Signed-off-by: Maxim Sukharev max@smacker.ru