Attention is currently required from: laforge, pespin, fixeria.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/32043 )
Change subject: logging: add 'logging timezone (localtime|utc)'
......................................................................
Patch Set 2:
(1 comment)
File src/core/logging.c:
https://gerrit.osmocom.org/c/libosmocore/+/32043/comment/bfc8215b_bd62f6de
PS1, Line 874: target->timezone = timezone;
> But we do not *need* to care whether a given
timezone function is available. […]
Re: it adds non-conforming API:
Ok granted, there could be rc in some functions.
Here it's simply my taste, I'd prefer to keep the look&feel of log_set_foo()
identical to the rest.
Re: it makes code more complex:
a limitation hidden in logging.c is duplicated to the function that sets the value. That
is bad for code maintenance, if something can stay in one place, it should.
So far I could work with your choice I guess, with a bit of hateful stare at the
"submit" button but ok =)
Re: harder to launch:
YES, harder to launch. This is not something that the user can fix, really. To have
gmtime_r() support, you need to move to a different operating system. So if you want to
share a config file between machines, you may have to take explicit care to disable the
irritating little completely unimportant timezone option for log output, just to be able
to launch the program. I don't want to add that.
If we're talking critical component, yes sure. Talking log output config: i don't
want to make this a deal breaking configuration choice.
Since I am firm on the last point, the rest above is tipped over: there would be no
callers using the rc. So I remain that this patch is fine as it is.
--
To view, visit
https://gerrit.osmocom.org/c/libosmocore/+/32043
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I7f868b47bf8f8dfcf85e735f490ae69b18111af4
Gerrit-Change-Number: 32043
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 14 Apr 2023 22:50:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment