-
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
Issue #566 maxFiles doesn't seem to behave as expected #578
Conversation
+1 @nimrod-becker could you add a test for this? |
Sure, actually there was a test especially for that case. However, the test always returned success (even though in fact there were 6 log files instead of three...). I've fixed the test, verified it's failing with the original code of file transport and working with the fix. |
} else { | ||
// The other files should be exist | ||
assert.doesNotThrow(function () { | ||
fs.statSync(fullpath); |
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.
Minor nit: 2-space tabs here.
Sorry about that :) fixed. |
@nimrod-becker will check it out again today and merge it if all is kosher. |
any news ? :) |
Works! 👍 |
Issue #566 maxFiles doesn't seem to behave as expected
Issue #566, when using maxFiles (with tailsble = false) deleting the oldest file (in _checkMaxFilesIncrementing) is not behaving as expected.
Calculating the target to delete, the log number should be (oldest !== 0 ? oldest : '') .