On Sun, Mar 25, 2012 at 12:17, Julian Foad <[email protected]> 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@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
