Skip to content

Conversation

@driftwoodstudio
Copy link

Updated class and constructor documentation to help others avoid two problems I experienced:

(a) not understanding how files are "grouped" using .identifier, and how this affects "grouping" of files when sending multiple Destinations to a single logging directory

(b) not understanding that omitting the .maxTimeInterval uses a (very short!) default of 10 minutes, and does not mean "no limit" as I had expected

DaveWoodCom and others added 6 commits February 4, 2020 06:21
Add documentation to explain how AutoRotatingFileDestination works, to help users understand the imporance of several configuration parameters.
Move text to describe the right parameter, as it was placed on the wrong one.
@driftwoodstudio
Copy link
Author

Note that my confusion with not understanding the internals of how "identify files for removing old archives" works caused me to assume it was based on file name patterns rather than the .identifier property. (Actually works internally by embedding the .identifier as an invisible file attribute of the files themselves, so was not obvious this existed by looking at log files in Finder.)

This confusion resulted in what seemed like unexpected behavior, and I opened issue #310 to report this. My problems were caused by (a) not specifying identifier and accepting default of "", and (b) assuming omitting the .maxTimeInterval meant "no limit" when it actually meant "10 minutes".

The core problem is with the defaults.

Because .identifier is so critical to the way AutoRotatingFileDestination rotation/purging works, I don't know if allowing a default value for .identifier at all is a good idea. It maybe should be a non-optional parameter.

More complicated is .maxTimeInterval. Here again I would argue for non-optional parameter. Omitting a value usually means "no value" but here it means a very specific and arbitrary value of "10 minutes". Making the parameter optional seems to imply "no value" or "unlimited" is an option, when in fact it's not.

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.

3 participants