subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julianf...@btopenworld.com>
Subject Re: New struct for holding (url, rev, repo-root) coordinates in client merge code
Date Tue, 03 Apr 2012 08:27:17 GMT
Greg Stein wrote:

> Julian Foad wrote:
>> ...
>> Sometimes we have more than two variables because we also want the repository root
URL, either to check it's the same repo as some other location or to derive the repo-root-relative
path (or fspath).  It's reached the point where it's definitely worth having a struct that
contains <url, rev, repo_root>.  Now we can write things like:
> 
> Please spell it like "repos_root_url" to be similar to precedent [...]


Done.  Currently:

{
  const char *repos_root_url;
  const char *repos_uuid;
  svn_revnum_t rev;
  const char *url;
}


> If you're going to have the repos_root_url in there, then I would
> recommend switching to repos_relpath rather than keeping a full URL.
> The relpath is being used within Ev2, and I'd like to see RA move over
> to <root, relpath> pairs for clarity/consistency (RA's arbitrary root
> and reparenting is very annoying and brittle).

Yes, I'd support switching to repos_relpath.  I think the use of this struct makes that easier
to achieve because now we carry around enough information to switch between one and the other
at any point without relying on external data (such as an RA session) that may be carried
around to some but not all of the functions that may want to do the conversion.

Because libsvn_client predominantly uses the full URL, we'll be best off sticking with URL
until the point at which most of the client APIs and code are using this struct, and then
we'll find that there are not many places left that actually care about the URL.  Some time
around that point we can gainfully switch to storing the relpath and creating the URL on demand.


>> It's not ideal as it is.  The pointer to a url_uuid_t sub-structure
>> is unnecessary at this stage (that is, unless and until we might
>> make the url_uuid_t structure into some bigger "repository"
>> abstraction); it would be simpler and therefore better right now to
>> embed the repo root URL and UUID directly.
> 
> Yeah. The substructure doesn't add any value, it seems.


Gone.

>>  Also, suggestions for a better name are welcome -- preferably
>> shorter, as it will have a 'svn_client__' prefix.
> 
> How about svn_client__pathrev_t, since it represents PATH@REV ?


That's not really lovely but it's better than what I've got.

I'll roll this out to libsvn_client private name space now.

- Julian

Mime
View raw message