Attention is currently required from: laforge, pespin, fixeria.

View Change

1 comment:

• File src/core/logging.c:

  • Patch Set #1, 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 change 32043. To unsubscribe, or for help writing mail filters, visit settings.