- Notifications
You must be signed in to change notification settings - Fork302
Declare implicit dependency on Six 1.9 or higher#301
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jayvdb commentedSep 15, 2016
I need this for#294. |
jayvdb commentedSep 15, 2016
It would be nice to have a test job that explicitly requires and uses six 1.4 , so that newer features are not inadvertently used without upgrading the requirement. |
amorde commentedSep 15, 2016
@jayvdb That sounds like a great idea, I'm not really sure how to go about doing that though. |
jayvdb commentedSep 15, 2016
You could add a job with env: - USE_OPTIONAL=true - USE_OPTIONAL=false+ - USE_SIX_14=trueAnd then in |
amorde commentedSep 26, 2016
@jayvdb It appears the minimum required version is actually1.9. Good call on the testing thing. I'll squash these commits and do a force push |
jayvdb commentedSep 27, 2016
In addition to the Travis errors described in#298, you are introducing many additional Travis jobs per build. This problem only requires one additional job. |
jayvdb left a comment
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.
Unnecessary jobs created.
jayvdb left a comment
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.
The commits probably need squashing.
.travis.yml Outdated
| env: | ||
| -USE_OPTIONAL=true | ||
| -USE_OPTIONAL=false | ||
| -USE_SIX_LOWEST_VERSION=1.9 USE_OPTIONAL=true |
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_SIX_LOWEST_VERSION is very long. Could be shorter.
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.
Changed toSIX_VERSION
amorde commentedOct 18, 2016
Hey@jayvdb, any updates on this? |
jayvdb commentedOct 18, 2016
amorde commentedOct 18, 2016
@jayvdb gotcha, haha sorry for the mixup! |
gsnedders commentedOct 27, 2016
Thanks for the reviewing,@jayvdb! :) Ultimately, we should almost certainly declare (and test) minimum required versions for all our dependencies. |
Uh oh!
There was an error while loading.Please reload this page.
This project uses a syntax for importing urllib.parse from six which was introduced in Six version 1.4
The import can be foundhere
The relevant change in Six can be foundhere
This can cause an issue if a project has a previous version of Six (say version 1.3) installed when installing html5lib
EDIT:
importing
viewkeysin html5parser.py requires six 1.9