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

Re: [PATCH][MERGE-TRACKING] Step 2 of repos to repos copyfrom info recording

From: Daniel Rall <dlr_at_collab.net>
Date: 2006-09-28 19:03:30 CEST

On Thu, 28 Sep 2006, Madan S. wrote:
...
> >>+get_copyfrom_mergeinfo(svn_ra_session_t *ra_session,
> >>+ apr_hash_t **copyfrom_mergeinfo,
> >>+ const char *rel_path,
> >>+ const char *copyfrom_path,
> >>+ svn_revnum_t src_revnum,
> >>+ apr_pool_t *pool)
> >
> >I think more of our APIs use an extra underscore in the
> >xxx_merge_info() function names:
> >
> > get_copyfrom_merge_info()
>
> I agree we have to be consistent in our naming. But a quick run though
> svn_mergeinfo.h tells me that none of our mergeinfo APIs use a _merge_info
> convention.

That's on the prefix, rather than function, portion of the name.

> I also ran a cscope search for the strings _mergeinfo and _merge_info
> (please note that this includes function declarations, calls and even doc
> strings containing the strings):
> _merge_info: 67 occurances
> _mergeinfo: 198 occurances

svn_mergeinfo.h and mergeinfo.c account for the discrepancy. Take a
look in libsvn_client/diff.c, and
libsvn_fs_util/merge-info-sqlite-index.c for the other side of the
equation.

> Personally I think that _ is an extra byte giving no extra information.
> Just like we mostly use copyfrom instead of copy_from. We could do away
> with it. Please let me know what you think.

I find the underscore to be more digestible.

> >>sizeof(copyfrom_mergerange));
> >...
> >>+/* Obtain the copyfrom mergeinfo and the existing mergeinfo of
> >
> >There should be a space between the words "merge" and "info" in the
> >doc string.
>
> doc string? Will this be visible when we generate the doxygen comments?

No.

> or do you just mean doc string (without implying doxygen).

Yes.

> Could you please tell me *how* this is important?
>
> btw, will do the change... just wanting to know the reason, so that I can
> use it for future judgement.

"Merge info" is two words. "mergeinfo" isn't a word. When first
encountering its concepts/reading doc strings, I find using real words
(rather than made-up) easier to understand.

...
> >>+ /* Find src path relative to the repos root */
> >
> >Do we have code to do this anywhere else?
>
> I dont think so. I checked again, and was not able to find equivalent code
> anywhere else.
> But, you are right... this is kinda generic. Should we make it an
> svn_path.h API?

Possibly! Propose an API signature and doc string.

  • application/pgp-signature attachment: stored
Received on Thu Sep 28 19:02:54 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.