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

move database initializer to new command #521

Merged
merged 3 commits into from Jan 14, 2018

Conversation

drujensen
Copy link
Member

Description of the Change

This addresses issue https://github.com/amberframework/granite-orm/issues/107

This moves the database initializer to new. There were several users that did not use generators and either copied their models over or created them by hand. The initializer was not generated for them so it created confusion.

Alternate Designs

The original thought was that if someone didn't want to use a database, then we shouldn't generate the initializer. However since the new command already assumes an MVC model, it makes sense that this initializer should be created on new instead of when a model is generated.

Benefits

This also eliminates some duplicate code between auth and migrate.

Possible Drawbacks

If the developer doesn't want to use granite or crectco, an extra file is generated that they will need to remove or change.

@drujensen drujensen requested review from a team January 14, 2018 02:20
Copy link
Contributor

@eliasjpr eliasjpr left a comment

Choose a reason for hiding this comment

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

Can't wait to see the Granite logger in the console. :)

@drujensen drujensen merged commit 7c92157 into master Jan 14, 2018
@drujensen drujensen deleted the dj/move-database-initializer-to-new branch January 14, 2018 17:43
@drujensen drujensen restored the dj/move-database-initializer-to-new branch January 14, 2018 17:50
@faustinoaq faustinoaq added this to Done in Framework 2018 May 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants