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

Fixes issue #2824 flask --version #2825

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@eso31
Contributor

eso31 commented Jun 11, 2018

Fixes issue 2824

Now when you run flask --version command it shows you the Werkzeug version as well. I changed the order as well to be Python, Flask and Werkzeug as it is shown in the issue template.

Let me know if it has to be done in some other way.

@emil45

This comment has been minimized.

emil45 commented Jun 16, 2018

It is best to remain consistent, the version of Flask imported in the global scope (line 28).
So werkzeug version should be also be imported in the global scope, rather than within the function.

As for the idea itself, I responded to you in issue.

@@ -21,6 +21,7 @@
from functools import update_wrapper
from operator import attrgetter
from threading import Lock, Thread
from werkzeug import __version__ as werkzeug_version

This comment has been minimized.

@cdvv7788

cdvv7788 Jul 17, 2018

What about importing werkzeug and referencing the version with werkzeug.__version__?
It is clear enough, and it is not as if it is being used anywhere else.

@@ -259,10 +260,11 @@ def locate_app(script_info, module_name, app_name, raise_if_not_found=True):
def get_version(ctx, param, value):
if not value or ctx.resilient_parsing:
return
message = 'Flask %(version)s\nPython %(python_version)s'
message = 'Python %(python_version)s\nFlask %(version)s\nWerkzeug %(werkzeug_version)s'

This comment has been minimized.

@cdvv7788

cdvv7788 Jul 17, 2018

Should .format be preferred over %?

This comment has been minimized.

@xavierhardy

xavierhardy Jul 23, 2018

Given the fact that it was originally there, I don't think so; it's not the point of this pull request. By the way, % is faster and more retro-compatible than .format.
https://hackernoon.com/a-closer-look-at-how-python-f-strings-work-f197736b3bdb

This comment has been minimized.

@cdvv7788

cdvv7788 Jul 23, 2018

Python2 is gonna die soon :P. F-strings are too recent.

This comment has been minimized.

@xavierhardy

xavierhardy Jul 23, 2018

Sure, then make a separate pull request, since most of this code uses % over .format.

~/src/flask$ find ./ -name "*.py" -exec grep -H "%" {} \; | wc -l
90
~/src/flask$ find ./ -name "*.py" -exec grep -H "\.format" {} \; | wc -l
37

But again, this is not the point of this pull request.

@pallets pallets locked as off topic and limited conversation to collaborators Jul 24, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.