Re: New struct for holding (url, rev, repo-root) coordinates in client merge code

On Sun, Mar 25, 2012 at 12:17, Julian Foad <julianfoad_at_btopenworld.com> 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
within libsvn_wc. We settled on "repos_root_url" and "repos_uuid" when
thinking about variable naming for WC-NG.

>...
> /* A location in a repository. */
> typedef struct repo_location_t
> {
>   url_uuid_t *repo;
>   svn_revnum_t rev;
>   const char *url;
> } repo_location_t;

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).

> 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.

>  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_at_REV ?

>...
>   * Replace the 'repo' field with a simple 'const char *repo_root_url' (and maybe a uuid as well), thus simplifying initial allocation and the 'dup' function.

Again: s/repo/repos/, just like the standard naming in WC.

>...

Cheers,
-g
Received on 2012-03-25 21:03:48 CEST