-
Notifications
You must be signed in to change notification settings - Fork 13
chore/update-click-8.0 #392
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
778d878
to
616b1da
Compare
setup.py
Outdated
@@ -32,7 +32,7 @@ | |||
python_requires=">=3.6.2, <4", | |||
install_requires=[ | |||
"chardet", | |||
"click>=7.1.1, <8", | |||
"click>=8.0.0, <8.1.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.
We don't need to have an upper-limit here. If someone is still running python 3.6 click>=8.1.0
simply won't get installed for them. And if 8.1.+ doesn't break anything in our code we don't need to artificially limit it if they are on 3.7+
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.
and we can also keep it >=7.1.1
since we're not depending on anything 8+ specific.
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 very cool
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.
our unit tests will fail on 7.1.1 by error message assertion but are we okay with that because it doesn't affect the actual user?
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.
We could just make the asserts be an or
, like
assert ("Could not open file: not_a_file" in result.stdout) or (" Invalid value for '--advanced-query': 'not_a_file': No such file or directory" in result.stdout)
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 have no idea why I didn't think of that
1e81479
to
561bd4d
Compare
Update click to latest major version >=8.0.0. Updated to
v8.0.4
. This major version was released in spring of 2021.It appears that the only changes that affected the CLI tests is trivial changes in error messages. That being said the list of changes is significant so I'm going to read through it carefully before release to make sure we didn't miss anything.
Worth noting that click 8.1.0 removes support for python <3.7 so we would have to up our python version requirement before further updating.