Skip to content
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

Updating extension guidelines v2 #1220

Merged
merged 8 commits into from
May 25, 2017
Merged

Updating extension guidelines v2 #1220

merged 8 commits into from
May 25, 2017

Conversation

untitaker
Copy link
Contributor

Continuing #1035 and #1218, #1135 related.

@jeffwidman
Copy link
Contributor

@untitaker - I'm unclear if you want to include updates to extension.rst or only extensiondev.rst in this PR...

I think extension.rst should also have a few updates, as it now conflicts with extensiondev.rst, leaving flask n00bs (such as myself) confused on how they should be importing flask extensions in the 1.0 release.

  1. extension.rst still suggests users should import using the deprecated flask.ext.foo rather than flask_foo:

If you have an
extension called Flask-Foo or Foo-Flask it will be always
importable from flask.ext.foo:

from flask.ext import foo

This conflicts with the recommendation in extensiondev.rst:

Flask extensions should urge users to import from flask_foo
instead of flask.ext.foo or flaskext_foo so that extensions can
transition to the new package name without affecting users.

I suspect (but not certain) that extension.rst should be changed to:

If you have an
extension called Flask-Foo or Foo-Flask you can import it from
flask_foo:

from flask_foo import foo

  1. If the above is updated, then the section Flask Before 0.8 reads a bit weird... the way the first sentence is currently worded implies the user will know about flask.ext.foo, even though all previous references to it have been removed.
    Since this is a major release, the cleanest thing might be to simply delete the entire "Flask Before 0.8" section. Alternatively, just have a simple note that says something along the lines of:

Deprecation note: Flask 0.8 deprecated flask.ext.foo. If you want to develop an
application that supports Flask 0.7 or earlier, see this note about using flaskext_compat.py.

Linking to the old docs should provide enough context on flask.ext.foo.

  1. Along those lines, should the script flaskext_compat.py be removed from the 1.0 release master branch? Neither that nor the flask-07-upgrade.py script seem necessary anymore since they're available through the maintenance branches.

  2. Should the following paragraph be deleted from extensiondev.rst?

Flask sets up a redirect package called :data:flask.ext where users should import the extensions from. If you for instance have a package called flask_something users would import it as flask.ext.something. This is done to transition from the old namespace packages. See :ref:ext-import-transition for more details.

  1. Should the following line be deleted from extensiondev.rst?

Flask 0.8 introduces a redirect import system that lets uses import from flask.ext.foo and it will try flask_foo first and if that fails flaskext.foo.

Leaving this in might accidentally encourage users to use flask.ext.foo as a catchall solution. I don't see any reason to leave it in, as the section reads fine without it.

I would happily submit a PR to implement the above changes except I'm not 100% confident on the proper solutions to these issues. Feel free to either implement changes or let me know the desired solutions and I'll fix them.

@untitaker
Copy link
Contributor Author

  1. extension.rst still suggests users should import using the deprecated flask.ext.foo rather than flask_foo

I agree that the wording implies the user should use flask.ext, but flask_foo doesn't actually import either Flask-Foo or Foo-Flask, it's just an ordinary package name. flask.ext.foo is the only name with such magic, and again, the magic is hard to maintain, which is one of the reasons we want to remove it.

  1. If the above is updated, then the section Flask Before 0.8 reads a bit weird... the way the first sentence is currently worded implies the user will know about flask.ext.foo, even though all previous references to it have been removed.
    Since this is a major release, the cleanest thing might be to simply delete the entire "Flask Before 0.8" section.

As mentioned in #1135, we need another migration tool for this. If we remove this section maybe more docstring text should be added to flaskext_compat.py.

  1. Along those lines, should the script flaskext_compat.py be removed from the 1.0 release master branch? Neither that nor the flask-07-upgrade.py script seem necessary anymore since they're available through the maintenance branches.

Not sure... if yes, the additional text mentioned above should be added in the maintenance branch. I'd say the change would be a bit radical, a safe thing to do is to rename it to make room for the new fixer. We also could extend the current compat tool so that it can do all kinds of conversions (the default being that it would convert flaskext.foo and flask.ext.foo to flask_foo).

  1. Should the following paragraph be deleted from extensiondev.rst?

    Flask sets up a redirect package called :data:flask.ext where users should import the extensions from. If you for instance have a package called flask_something users would import it as flask.ext.something. This is done to transition from the old namespace packages. See :ref:ext-import-transition for more details.

  2. Should the following line be deleted from extensiondev.rst?

    Flask 0.8 introduces a redirect import system that lets uses import from flask.ext.foo and it will try flask_foo first and if that fails flaskext.foo.

Leaving this in might accidentally encourage users to use flask.ext.foo as a catchall solution. I don't see any reason to leave it in, as the section reads fine without it.

In general i'd rather just convert information to the past tense instead of deleting it... maybe we could move all information about flask.ext into just one chapter, and link this chapter sometimes for historic reasons, but not leave mentions of flask.ext scattered.

@untitaker
Copy link
Contributor Author

Oh, and i think any such changes should go in a separate PR pointed to master.

Flask extensions should urge users to import from ``flask.ext.foo``
instead of ``flask_foo`` or ``flaskext_foo`` so that extensions can
Flask extensions should urge users to import from ``flask_foo``
instead of ``flask.ext.foo`` or ``flaskext_foo`` so that extensions can

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

jeffwidman and others added 8 commits June 6, 2015 04:43
According to #1092 (comment) and #1085 (comment) , the correct order to attempt to import extensions should be flask_foo, then flask.ext.foo, then flaskext_foo.
pip doesn't install links included in the description of projects
anymore. Therefore ==dev install doesn't work anymore.
Python 2.6 is not supported by python-dev anymore and does not get any
security updates. Even though Flask supports 2.6 at the moment, I think
it's not necessary for any extensions that are going to be approved in
the future to support 2.6.
Flask and several extensions already supports Python 3.3 and higher. By
requiring approved extensions to support Python 3.3 as well we can
quickly achieve better Python 3 adoption and make using Python 3 easier
for users.

The effort of supporting both Python 2.7 and Python 3.3 is small enough
that it shouldn't be a problem to require this from extension authors.
@untitaker untitaker force-pushed the ext-deprecation branch 2 times, most recently from a61ed9f to 67d7fc3 Compare June 6, 2015 02:48
@untitaker
Copy link
Contributor Author

This is a complete mess and doesn't merge anymore, we should cherry-pick out of it and kill it then.

@@ -410,8 +404,8 @@ deprecated ``flaskext.foo``. Flask 0.8 introduces a redirect import
system that lets uses import from ``flask.ext.foo`` and it will try

This comment was marked as off-topic.

@jeffwidman
Copy link
Contributor

Taken care of in #1484

@jeffwidman jeffwidman closed this Apr 5, 2016
@jeffwidman jeffwidman deleted the ext-deprecation branch April 5, 2016 09:16
@untitaker untitaker restored the ext-deprecation branch April 5, 2016 16:51
@untitaker
Copy link
Contributor Author

Not really, we still need to update the extension guidelines in some way.

@untitaker untitaker reopened this Apr 5, 2016
@jeffwidman
Copy link
Contributor

My bad.

Can you clarify what's missing?

005a49d updated the extension guidelines and 188f0e2 added a deprecation warning.

Is there something else beyond the switch to flask_extensionname?

@untitaker
Copy link
Contributor Author

@jeffwidman Yes, I think the enumerated list for extension requirements has to be updated as well. That is a completely unrelated change, see #1035

@kennethreitz
Copy link
Contributor

needs a rebase!

@kennethreitz kennethreitz merged commit f80ea4f into master May 25, 2017
@kennethreitz
Copy link
Contributor

i merged this manually.

@kennethreitz kennethreitz deleted the ext-deprecation branch May 25, 2017 21:22
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants