-
Notifications
You must be signed in to change notification settings - Fork 68
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
feat: improved support of python3 #17
Conversation
7697ed5
to
f90ed5c
Compare
With modern python running old instruction will trigger a warning: ``` setup.py:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives ``` To avoid this (and other related warnings) were updated: - package `distutils.core` replaced by modern `setuptools` - updated README.txt to use PIP instead call setup.py directly - used python3 in README.txt instead python That gives 2 extra features beside rid of warnings: - you can easily install dash_hash package without root permission on host - you can uninstall package now by pip3/python-pip easily
f90ed5c
to
573ecc1
Compare
573ecc1
to
0dab324
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.
ACK
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.
utACK for squash merge; strong concept ack :) @thephez some docs will need updating
@PastaPastaPasta This is the only reference I see in the dash repository: https://github.com/dashpay/dash/tree/master/test#dependencies-and-prerequisites. If you're aware of others, please share. |
Originally posted by @thephez in #17 (comment) I'll prepare PR for that repo too |
@knst You can build off dashpay/dash#5291 if more changes are needed |
Hmm, this is going to break too? https://github.com/dashpay/dash/blob/develop/contrib/containers/ci/Dockerfile#L47 |
since this is a breaking change it might make sense to bump version too (1.3.3 or 1.4) |
Last changes are breaking changes
As I just tested, upgrade from version 1.3.2 also works:
|
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.
👍
utACK
## Issue being fixed or feature implemented Install of dash_hash will change once dashpay/dash_hash#17 is merged ## What was done? - Changed install instructions to match new install in dash_hash README - Updated Dockerfile to install correctly ## How Has This Been Tested? N/A ## Breaking Changes None ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation
## Issue being fixed or feature implemented Install of dash_hash will change once dashpay/dash_hash#17 is merged ## What was done? - Changed install instructions to match new install in dash_hash README - Updated Dockerfile to install correctly ## How Has This Been Tested? N/A ## Breaking Changes None ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation
With modern python the setup process by an old instruction will trigger a warning:
setup.py:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
To avoid this (and other related warnings) were updated:
distutils.core
replaced by modernsetuptools
That gives 2 extra features beside rid of warnings: