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

Updated swift documentation for AutoRotatingFileDestination #311

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

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