So, here is some working code along the lines I mentioned. Since interval_to_seconds() lives in OpenSRF, the meat of this version is actually an OpenSRF branch:
There may be more places to use this new skill, but these are the ones highlighted in the discussion so far.
This works pretty well for me, but I do wonder a few things:
1) The new behavior is completely optional. If we feel it is *generally* more correct behavior, we could consider reversing that, and making it the default with a context of "now()", with a compatibility switch for "simple"-version calculations. That would obviously be more disruptive short-term, but if it pays off long-term, maybe worth it.
2) It would be pretty simple to sub out some of the logic to a new "interval_to_duration" function in OpenSRF::Utils. This current branch is great for simplicity of the upper layer changes (and we could keep that as an option), but forcing it all down to seconds is probably some unnecessary work in many cases. We could instead use this new duration directly for the math, and everything would work fine. I think that would actually be truest to the spirit of the original discussion/branch, but the round-tripping of the interval tripped (*ahem*) things up a bit.
I'd like to go ahead and try extracting the "interval_to_duration()" sub-layer if we go this direction, but also wanted to get some "early" feedback on this overall. Thanks!
Sorry for the delay.
So, here is some working code along the lines I mentioned. Since interval_ to_seconds( ) lives in OpenSRF, the meat of this version is actually an OpenSRF branch:
http:// git.evergreen- ils.org/ ?p=working/ OpenSRF. git;a=shortlog; h=refs/ heads/user/ dbwells/ lp1635737_ interval_ to_seconds_ contextual
With that in place, the EG changes are of course pretty simple:
http:// git.evergreen- ils.org/ ?p=working/ Evergreen. git;a=shortlog; h=refs/ heads/user/ dbwells/ lp1635737_ better_ interval_ to_seconds
There may be more places to use this new skill, but these are the ones highlighted in the discussion so far.
This works pretty well for me, but I do wonder a few things:
1) The new behavior is completely optional. If we feel it is *generally* more correct behavior, we could consider reversing that, and making it the default with a context of "now()", with a compatibility switch for "simple"-version calculations. That would obviously be more disruptive short-term, but if it pays off long-term, maybe worth it.
2) It would be pretty simple to sub out some of the logic to a new "interval_ to_duration" function in OpenSRF::Utils. This current branch is great for simplicity of the upper layer changes (and we could keep that as an option), but forcing it all down to seconds is probably some unnecessary work in many cases. We could instead use this new duration directly for the math, and everything would work fine. I think that would actually be truest to the spirit of the original discussion/branch, but the round-tripping of the interval tripped (*ahem*) things up a bit.
I'd like to go ahead and try extracting the "interval_ to_duration( )" sub-layer if we go this direction, but also wanted to get some "early" feedback on this overall. Thanks!