bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
797 stars 187 forks source link

Fixed colorStatus / Added colorize option / Using lodash #109

Closed brunolemos closed 8 years ago

brunolemos commented 8 years ago

Fixed: colorStatus was not working at all. Fixed #86: Now expressFormat will have color or not based on colorize option.

rosston commented 8 years ago

This is a large diff with significant changes. I think there are simply too many changes here for one PR. Each of these could be their own:

I could see that last change alone requiring a major version bump.

brunolemos commented 8 years ago

The only "breaking" change is that the expressFormat is now without colors by default, depending on the colorize parameter. But I believe that it can be easily changed with a new commit, if necessary. Hope it gets accepted.

PS: The colorStatus parameter still exists. Just updated the docs to reflect it.

floatingLomas commented 8 years ago

Ya, that's a huge bunch of changes for one PR; the big trigger for me is that if I ask you to 'squash your changes,' there are just too many changes in what becomes a single commit. This really should be four PRs (or three as pointed out, because #107).

If it introduces a breaking change, I agree with @rosston that it would need a major version bump.

Also of note, last time winston bumped major versions, we did too (#81).