-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Reverse log level order for all levels so that syslog can be used again #290
Conversation
Any idea if this can be merged in? |
@mheap Could you add a test to prevent regression as this was already once broken before? |
@3rd-Eden Had to do a little refactoring to get the data I needed out, but I've added three tests, one for each supplied set of config levels |
@mheap It seems that some of your changes causes the test suite to fail on Node 0.6 |
Runs fine for me on 0.6.21 locally. According to Travis, the build stalled rather than failing. Edit: Just spotted that it was restarted 2 hours ago. Not sure what's going on there. |
@3rd-Eden Any ideas what might be causing the build to stall on Travis? |
@3rd-Eden Ping. Can we merge this anyway? It's a blocker for a lot of people that are using syslog levels |
@mheap I'm not merging this in until the tests are passing. I simply don't want new code to break the library as those tests we're added for a reason. |
@3rd-Eden Any ideas why they're stalling on Travis? I can run them fine locally with the exact version that Travis uses |
@3rd-Eden Interestingly, it builds fine for me - https://travis-ci.org/mheap/winston/jobs/10511961 The Node 0.8 tests stall instead, though Edit: Interestingly, the last test run is the same both times: " An instance of the Daily Rotate File Transport after the logs have flushed the query() method using a bad from and until option |
@3rd-Eden Tests seem to be passing now on Travis. No change to the code, but as the error was that it was stalling and I couldn't reproduce locally, I'm not convinced there was ever an issue there My test run is located at : https://travis-ci.org/mheap/winston |
Any progress on this? |
@julien51 I can't reproduce the travis stalling issue consistantly. It looks like the tests just hang (I think it's to do with an early return if some setup fails, that doesn't call the callback) but I haven't had time to really dig into it |
Dammit. Is stalling just for 0.6 ? How far back does winston has to support? |
Hum. At this point, even |
@indexzero It's been a while, but I just triggered another build of this and the tests all passed. I think it was to do with some test setup previously (the failing tests were nothing to do with my changes). As it's passing now, any chance of getting it merged? |
cc @3rd-Eden on this PR, now with tests passing |
This has been resolved in the |
And thanks again for your contribution! |
Fixes #249. As syslog level numbers now conform to the RFC, we need to reverse the order of the NPM and CLI loggers too, as well as the comparison which checks levels before logging