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

Update config extend to allow custom logger #3

Merged

Conversation

ruimarinho
Copy link

This allows using a config.js file with a custom logging function defined.

@ruimarinho
Copy link
Author

@sdepold I'm having trouble setting up a test case for this. It doesn't seem to fit in migrate.test.js yet it needs most of its boilerplate code from prepare.

@sdepold
Copy link
Member

sdepold commented Jun 3, 2014

@ruimarinho the tests are currently still in a pretty shitty shape and needs to be refactored. I will do this during the next days/week. For now, feel free to just copy and paste the prepare block.

@sdepold
Copy link
Member

sdepold commented Jun 3, 2014

plus: good catch!

@ruimarinho
Copy link
Author

I tried to, but I think I'll wait for the refactor because right now it's pretty complicated to get around the tests. I'm a big advocate for gulp, but do you think it's the right approach in this case? I've had nothing but good experiences with a thin cli based off individually tested library code.

@sdepold
Copy link
Member

sdepold commented Jun 4, 2014

Not sure if I understand you :D We discussed yesterday, that we will move the gulp tasks itself into a separate project and just use the tasks then in the cli

@sdepold
Copy link
Member

sdepold commented Jun 13, 2014

Tbh. I actually wonder how you would ever configure the logging as you cannot define a function within json... Anyway, I wrote a test for it and merged the PR.

@sdepold sdepold merged commit 569dec6 into sequelize:master Jun 13, 2014
@ruimarinho ruimarinho deleted the enhancement/db-options-custom-logger branch June 23, 2014 16:01
@ruimarinho
Copy link
Author

@sdepold sorry for the delay, I was on vacations. I'm using a js configuration file, not a json one :) what I meant regarding gulp was that maybe the tests are too low-level by testing stdout. If you had tested functions return values, maybe it'd be easier to test them like a regular script?

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