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

Added: sqllite #278

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Added: sqllite #278

wants to merge 7 commits into from

Conversation

frank-dspeed
Copy link
Contributor

No description provided.

Copy link
Contributor

@justinbmeyer justinbmeyer left a comment

Choose a reason for hiding this comment

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

Will this work in production? Also, can you update the readme with whatever people will have to install locally? Where does sqlite come from?

@@ -1,15 +0,0 @@
var async = require('async');
Copy link
Contributor

Choose a reason for hiding this comment

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

why can these migration files be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I merged the migrations files the removed migration files contained updates to filds and colums i integrated this changes directly into the main migration file that creates the filds and colums.

i did that because sqlite don't support this migration methods at present but it has still all included all is working its realy well tested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this will also work in production! with diffrent drivers also you only need to adjust the database.json.

sqllite comes from i don't know i saw it was in already maybe from some tests or that but its in.

the database gets created if a driver in database.json with a filename is set via the driver this gets done in the bookshelf.js file it self.

hope all that makes sense.

Conclusion Migrations Cleaned up.
Bookshelf model Loader Modifyed to work with filebased knex drivers also.
database.json moved postgres example to test and replaced dev env setting with sqlite

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justinbmeyer oh and most importent thats why we have automated tests :) they all pass :)

@frank-dspeed
Copy link
Contributor Author

frank-dspeed commented Dec 17, 2016

@justinbmeyer question about readme.md changes there are now no additional settings needed any more as the database gets created by the model loader if it is not there it runs directly out of the box after npm install. without adjusting database.json dev gets used. in production it still uses what ever is supplyed as database host string this could be pointed out that if you want to use sqllite also in production simply adjust database.json thats all.

what do you think

Copy link
Contributor Author

@frank-dspeed frank-dspeed left a comment

Choose a reason for hiding this comment

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

  • Migrations Cleaned up. merged additional changes with inital creation
  • Bookshelf model Loader Modifyed Extended to work with filebased knex drivers also.
  • database.json Modifyed moved postgres example to test env and replaced dev env setting with sqlite

@@ -1,15 +0,0 @@
var async = require('async');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@justinbmeyer oh and most importent thats why we have automated tests :) they all pass :)

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