On May 22, 2008, at 6:29 AM, Mark Phippard wrote:
> On Thu, May 22, 2008 at 1:36 AM, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
>> Mark Phippard wrote:
>>>
>>> I have a question on recent JavaHL change merged to 1.5.x branch.
>>>
>>> In the logMessageCallback we now return a Map of revprops instead of
>>> the individual revprops. svn:date is returned as the date value.
>>> This forces us to add code like the following to get the timeMicros:
>>>
>>>
>>> // Really hacky date parser, because Java doesn't support
>>> // microseconds natively.
>>> try {
>>> DateFormat formatter = new SimpleDateFormat(
>>> "yyyy-MM-dd'T'HH:mm:ss.SSS");
>>> String datestr = ((String) revprops.get("svn:date"));
>>> Date date = formatter.parse(datestr.substring(0, 23));
>>> Calendar cal = Calendar.getInstance();
>>> cal.setTime(date);
>>> timeMicros = cal.getTimeInMillis();
>>>
>>> timeMicros = timeMicros * 1000
>>> +
>>> Integer.parseInt(datestr.substring(23,
>>> 26));
>>>
>>> I thought the timeMicros were only added in 1.5 to be a more
>>> efficient
>>> way to pass the date along. Or perhaps it was the extra precision.
>>> Either way, the efficiency and precision are both gone now. It
>>> seems
>>> like we should either:
>>>
>>> a) pass the timeMicros as an explicit parameter to the callback
>>> b) store it in the revprops. Either as the value of svn:date or
>>> as a
>>> made up revprop like svn:timemicros.
>>> c) forget it altogether and just pass along the date as is
>>
>> I don't recall when timeMicros was added (but the fact that I
>> needed to
>> parse the svn:date property for compat probably means it wasn't in
>> 1.5).
>
> Check the branch:
>
> http://svn.collab.net/repos/svn/branches/1.4.x/subversion/bindings/java/javahl/src/org/tigris/subversion/javahl/LogMessage.java
>
> timeMicros is not there.
>
>> As
>> for your suggestions, I'm opposed to a) on the grounds that we
>> shouldn't be
>> treating svn:date as special in this callback. We could do
>> something like
>> b) relatively easy (though I'd prefer to do it as an additional
>> property,
>> not as svn:date). c) would be easiest, and ideal if it still
>> provides
>> consumers what they need.
>
> So a possible option c) would be to remove the timeMicros stuff
> altogether and just go back to having a Date. We can wait for Blair
> to comment. It seems to me that if the original goal was efficiency
> we definitely no longer have that.
No, the goal was to not have lossy dates. I believe if you get the
date/timestamp from the 1.4 bindings for a revision and then lookup
the revision with it, the revision will be - 1 less then the original
revision, so its lossy.
I am -1 on removing the time micros, I am +1 on putting timeMicros
back in as an explicit paramter and -0 on the hacky parameter name.
I think the best solution is to put back explicit parameters for all
our supported svn:* revprops and use the map for anything else. The C+
+ code can easily remove the svn:date, svn:log and svn:author from the
map.
Blair
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-22 16:15:03 CEST