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