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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-CC: fixeria <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Fri, 14 Apr 2023 22:50:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <[email protected]> Comment-In-Reply-To: laforge <[email protected]> Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
