[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Issue #2287 - Make svn_client_log() take a peg revision

From: S.Ramaswamy <srsy70_at_gmail.com>
Date: 2005-08-10 17:38:59 CEST

Sorry for the delay.

>
> I meant that, but more strongly: I meant the pair of doc strings are
> incomplete. If log2 depends on "unspecified" having a particular meaning in
> log3, then either that meaning must be documented publically for log3 or it is
> an implementation detail that shouldn't be mentioned publically for log2. In
> the former case, it would be fine for log2 to be described in terms of what
> value it passes to log3; in the latter case, log2 would have to be described in
> terms of its effect rather than that value which would be an implementation detail.
>

Ok, I could expand the doc string for svn_client_log3(), to include:

 * @a peg_revision indicates in
 * which revision @a path_or_url is valid. If @a peg_revision is @c
 * svn_opt_revision_unspecified, then it defaults to @c
 * svn_opt_revision_head for URLs or @c svn_opt_revision_working for
 * WC targets.

and the doc string for svn_client_log() would remain the same:

Note that svn_client_checkout2(), svn_client_checkout have similar doc
strings w.r.t peg revision.

> Now that I write and think about this clearly enough to present those two
> options, I wonder whether log3 needs to accept "unspecified" as part of its
> publically documented interface. It would be better if it didn't, unless there
> is some good public reason to do so. We have too many unnecessary defaults
> already in APIs like this; we shouldn't add more unless there is a clear
> benefit. It could still accept "unspecified" privately to facilitate the
> implementation of log2.
>
> (When I said, "I wasn't easily able to determine the meaning for myself," I
> meant that I wasn't able to determine the meaning of "unspecified", even by
> looking at the implementation. It certainly wasn't possible from the doc strings.)
>
>
> >>>+ if (start->kind == svn_opt_revision_number &&
> >>>+ end->kind == svn_opt_revision_number)
> >>>+ revision = (start->value.number > end->value.number) ? *start : *end;
> >>>+ else
> >>>+ revision.kind = svn_opt_revision_unspecified;
> >>>
> >>>+ SVN_ERR (svn_client__ra_session_from_path (&ra_session, &rev,
> >>>+ &url_p, path,
> >>>+ peg_revision, &revision, ctx,
> >>>+ pool));
> >>>+
> >>
> >>Please could you explain this change. I don't understand why something special
> >>is happening if both the start and end revisions are specified as numbers
> >>rather than, for example, dates. And I don't really understand _what_ special
> >>behaviour is happening in that case.
>
> I'd still like you to explain this part of the patch if you would, please.
>

From the doc string for svn_client__ra_session_from_path(), revision is,
"a desired revision REVISION, create an RA connection to that object as it
exists in that revision, following copy history if necessary."

I meant to get a ra_session to the youngest revision of the object, before
using the operating revisions on that.

> >>I see from the implementation of svn_client__ra_session_from_path() that an
> >>unspecified "revision" means "revision = peg_revision", so wouldn't it be
> >>clearer to make your "else" clause say "revision = *peg_revision" (or an
> >>equivalent pointer assignment) ?
> >

only if the peg revision->kind is not unspecified.

> >
> > Revision numbers can also be specified as a combination of dates and
> > revision numbers, as well as a combination of keywords (PREV, HEAD, etc),
> > numbers etc. In those cases the revision gets set to
> > svn_opt_revision_unspecified in the current patch(v2).
>
> Yes, but _why_ do you want it set to "unspecified" in such cases? :-) (Maybe
> it's really obvious, but not to me. Maybe I just haven't studied it hard enough.)
>

 When both revision numbers are specified as numbers, then
it is easy to compare. The revision number corresponding to revision keyword
like HEAD or a date revision can be determined only after you get a session
object with svn_client__get_revision_number(), svn_ra_get_dated_revision()
etc.

Please let me know if my understanding is correct or if there is
better way to do this.

Thanks
Ramaswamy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 10 17:40:01 2005

This is an archived mail posted to the Subversion Dev mailing list.