-
Notifications
You must be signed in to change notification settings - Fork 11
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
Set minimal click version to 8.0 #30
Conversation
@vznncv Thank you ++ fort this. |
The pull request #27 adds `update_min_steps` option usage of click._termui_impl.ProgressBar that is available from click 8.0. So corresponding requirements should be added to setup.cfg. Signed-off-by: Konstantin Kochin <vznncv@gmail.com>
Done |
Thanks. Let me think a second to see if we can support both versions as Click is otherwise rather popular and I do not want us to also break scancode-toolkit usage as a library. |
I am running a local test first with this:
|
And at the moment Celery which is a dep of ScanCode.io does not (yet) support Click 8+ |
I ran these tests: and all pass with this patch:
The lines
... are duck typing at its best... Not super elegant but effective nonetheless! So with this patch we can accept all currently supported Click versions and not only > 8 |
I'm closing this pull requests since @pombredanne has proposed better solution. @pombredanne Did you consider tox usage instead of |
@vznncv let me keep this open for now. Tox in addition would help! +1 Note that ./configure does a setup that's also useful beyond testing |
Closing for aboutcode-org/skeleton#36 |
And #31 |
The pull request #27 adds
update_min_steps
option usage ofclick._termui_impl.ProgressBar
that is available from click 8.0. But existedsetup.cfg
contains the following restriction:click >= 6.7, !=7.0
. It breaksscancode
withclick
<8 version installation. For example: https://github.com/ARMmbed/mbed-os/pull/14981/checks?check_run_id=3470879205This pull request updates minimal
click
version in thesetup.cfg
according changes in the pull request #27.