Blair Zajac wrote:
> C. Michael Pilato wrote:
>> Hyrum K. Wright wrote:
>>>>> 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.
>>>
>>> Does the command line client have this same problem? It would seem
>>> that all the date information is recoverable from svn:date. What
>>> part is being lost?
>>
>> Yeah, I'm interested in this, too.
>>
>>>> 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.
>>>
>>> True, that it's easy for the C++ code to filter out svn:* revprops,
>>> but I don't see why we should. Getting the property values from a
>>> Map isn't difficult, and it's much more extensible than passing
>>> values back directly. Also, I'd like to keep the JavaHL interface
>>> from drifting to far away from the C interface.
>>
>> I don't use the Java bindings directly, but +1 on the sentiment of
>> keeping them from drifting unnecessarily from the C interface. I
>> don't see how returning svn:date's value asis would be "lossy". I
>> mean, if it is, it would seem we certainly couldn't remedy that
>> client-side. Why not just keep Hyrum's changes, but provide a handle
>> utility function that converts Subversion dates into timeMicros?
>
> Looking more closely at the code:
>
> 1) Only the java.util.Date is lossy, it only represents times to the
> millisecond, so I put in the extra methods in LogMessage so you can get
> the time in micros without doing any extra work.
Would it be possible to have a LogDate class which extends
java.util.Date (and can therefore be passed around as such), but which
isn't lossy?
> 2) The LogMessage class is already not similar to the C interface in
> that it provides a strongly typed date as a java.util.Date. My work
> just extended it to add a long microseconds values.
See note farther down about LogMessage being deprecated.
> 3) The current LogMessage class doesn't have the non-svn revision
> properties in it, which I believe should be the real point of this
> work. Looking at the current interface, it only exposes the svn primary
> properties through methods (getAuthor, getMessage, etc), there's no way
> to get the map of other properties.
>
> 4) I don't see the point of moving the svn:date string to time parsing
> from the C code to the Java code, it's probably slower.
Higher level languages are usually easier to maintain. Generally, I've
tried to keep any JavaHL specific stuff in the highest layer possible.
That's why all the compat wrappers are written in Java, instead of C++.
Also, I don't think the speed increase would be noticeable, but I
haven't run any tests, have you?
> So I think we can have the best of both worlds.
>
> 1) LogMessageCallback can have its String author, long timeMicros,
> String message arguments returned and keep the of Map of revprops as an
> argument. Put the time conversion back in the C code and remove the
> time parsing from the Java code.
>
> 2) LogMessage can grow a Map<String,String> field and a
> getRevisionProperties() method for getting all revision properties.
LogMessage is (or should be) deprecated, since it is only used by
deprecated APIs. We shouldn't be adding anything to it.
> We can either remove svn:date, svn:author and svn:log from the revprops
> map or keep them in. This is a style issue. Maybe keeping them all in
> is better? This seems more consistent with the meaning of the map???
> And the meaning of the C code where the map has all properties.
>
> So we can take Mark's patch and revert parts of the C++ code in r31273
> to get the desired code.
If we do include explicit parameters to the callback method, what would
we return as svn:author, svn:date, and svn:log if the caller doesn't
include those in the list of revprops to be fetched? The logMessages()
interface is now an arbitrary revprop request mechanism, not just a way
of getting log messages, which is a good reason for not giving the svn:*
revprops special treatment.
It looks like the svn:date value is the major problem here. If we are
concerned about date parsing, let's just provide a utility function for
interested clients and call it good.
-Hyrum
Received on 2008-05-23 08:35:16 CEST