-
Notifications
You must be signed in to change notification settings - Fork 136
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
Allow all of Sequelize options in config.sequelize
#5
Conversation
http://docs.sequelizejs.com/en/v3/api/sequelize/#new-sequelizedatabase-usernamenull-passwordnull-options for example: ```js exports.sequelize = { port: '3306', host: 'localhost', username: 'root', password: '', database: 'test', dialect: 'mysql', pool: { max: 5, min: 0, idle: 10000, }, storage: 'db/test-foo.sqlite', timezone: '+08:01', }; ``` Do not assert config.host, password, port config, let them has default value. Do not assert config.database, because SQLite case not need it.
@huacnlee, thanks for your PR! By analyzing the history of the files in this pull request, we identified @popomore, @jtyjty99999 and @luicfer to be potential reviewers. |
Codecov Report
@@ Coverage Diff @@
## master #5 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 5
Lines 39 35 -4
=====================================
- Hits 39 35 -4
Continue to review full report at Codecov.
|
lib/loader.js
Outdated
@@ -16,30 +15,18 @@ Sequelize.prototype.log = function() { | |||
module.exports = app => { | |||
const config = Object.assign(app.config.sequelize, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
应该是调转来,app.config.sequelize 去 merge 默认的
我发 patch |
@jtyjty99999 我发不了,只能你来发。 |
npm owner 加一下我们吧 |
修改 Changelog |
@huacnlee changelog 在 release 的时候用命令生成的 $ git changelog -n -t 1.1.0
$ vim package.json # change `version`
$ git release 1.1.0
$ npm publish |
|
http://docs.sequelizejs.com/en/v3/api/sequelize/#new-sequelizedatabase-usernamenull-passwordnull-options
Issue from: eggjs/egg#341
for example:
Checklist
npm test
passesAffected core subsystem(s)
Description of change