Closed felfert closed 5 years ago
I like the use of tzlocal, this makes things easier. What I'm not sure is having it as a "hidden option" and passing it through to the formatting functions.
What would be of benefit of doing this rather than calling get_localzone()
where it's needed (i.e. log_entry_header_from_commit()
and log_entry_header_from_tag()
)?
Well, invoking get_localzone() only once prevents race conditions and is more performant. What if during formatting of the log entries, the admin (in a different session) changes the timezone (which is just a symlink /etc/localtime)?
If you don't like handing down in options, we can use a global var but that's ugly.
Well, invoking get_localzone() only once prevents race conditions and is more performant. What if during formatting of the log entries, the admin (in a different session) changes the timezone (which is just a symlink /etc/localtime)?
I wouldn't consider performance the critical factor (it's not exactly calculating prime numbers, so "premature optimization as the root of all evil, yada yada"), yet the argument about changing timezone (unlikely yet not impossible) is definitely a valid one.
If you don't like handing down in options, we can use a global var but that's ugly.
I'm not a huge fan of passing down options mostly because of mixing the user parameters with additional data; since header formatting functions don't need all this baggage at this point - then I'd rather keep the interface clean. The argparse.Namespace is mightily useful... and also kind of messy.
How about passing in just the timezone?
Have created a new timezone branch, based on current master and implementing the requested changes. See new PR #44
Fixes #4 Using tzlocal, because dateutil.tzlocal is broken.
Notes: