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

Re: svn commit: r31874 - in trunk/subversion: include libsvn_client svn tests/cmdline

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Mon, 30 Jun 2008 12:41:27 -0400

Karl Fogel wrote:
> cmpilato_at_tigris.org writes:
>> --- trunk/subversion/libsvn_client/mergeinfo.c (r31873)
>> +++ trunk/subversion/libsvn_client/mergeinfo.c (r31874)
>> @@ -1118,13 +1118,55 @@ logs_for_mergeinfo_rangelist(const char
>> return SVN_NO_ERROR;
>> }
>>
>> +static svn_error_t *
>> +location_from_path_and_rev(const char **url,
>> + svn_opt_revision_t **revision,
>> + const char *path_or_url,
>> + const svn_opt_revision_t *peg_revision,
>> + svn_client_ctx_t *ctx,
>> + apr_pool_t *pool)
>> +{
>
> Doc string.
>
> Seriously. That function may look obvious, but all sorts of questions
> can arise: what are the legal values in the substructure fields in
> *peg_revision, and how do they behave? May url or revision be NULL if
> the caller doesn't care about that information? Are there any
> particular error conditions it's important to promise? Etc.
>
> I recently spent more time reviewing the r31059 group in 1.5.x/STATUS
> than I should have needed, because the get_full_mergeinfo() internal
> function was not -- and still is not -- documented. Let's please not
> perpetuate this problem :-).

Nono, I totally agree. For some reason docstrings on helper functions have
slipped routinely out of scope in my review-before-commit stuff. Thanks for
calling attention to this.

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on 2008-06-30 18:41:42 CEST

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.