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

add example of how to set up with init_app method #302

Merged

Conversation

wgwz
Copy link
Contributor

@wgwz wgwz commented Dec 5, 2019

No description provided.


from flask_migrate import Migrate

migrate = Migrate()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd want the db also initialized up here.

db = SQLAlchemy()
migrate = Migrate()

docs/index.rst Outdated

def create_app():
...
# instantiate your db
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be "initialize" your db.

docs/index.rst Outdated
...
migrate.init_app(app, db)
...
# import your models
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The models do not necessarily need to be imported here, and a lot of people prefer to not do imports inside functions, so I would remove this comment and leave it up to the developer to decide where to do the import.

@miguelgrinberg
Copy link
Owner

Thanks for the addition, Kyle. Take a look at my comments and let me know what you think.

@wgwz
Copy link
Contributor Author

wgwz commented Dec 5, 2019 via email

@wgwz
Copy link
Contributor Author

wgwz commented Dec 6, 2019

Made those edits and I added a docstring for the create_app function that says "Application-factory pattern". My reasoning is that this will give newcomers a term to search for if they need more info.

@miguelgrinberg miguelgrinberg merged commit 6a76c24 into miguelgrinberg:master Dec 6, 2019
@miguelgrinberg
Copy link
Owner

Thanks, Kyle!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants