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

Re: svn commit: r28768 - in branches/issue-2897/subversion: libsvn_client tests/cmdline

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Tue, 08 Jan 2008 21:22:51 +0530

>> apr_array_header_t *reflected_ranges;
>>
>
> I'd like to see a newline here.
>

Fixed in r28784.

>> + opt_revision2.value.number = range->end;
>> + SVN_ERR(svn_client_diff_summarize_peg2(target_url, &peg_revision,
>> + &opt_revision1, &opt_revision2,
>> + depth,
>> + merge_b->ignore_ancestry,
>> + get_diff_summary_func_cb,
>> + merge_b->reflective_rev_affected_paths,
>> + merge_b->ctx,
>> + iterpool));
>>
>
> Level of indentation looks off here.
>

Posted to dev list seeking for how to format the longish_func_call with
very long parameter.

> This is also going to be a potentially really expensive operation, and should
> be documented as such in the API's doc string.
>
>

I agree it is expensive, we run such summaries *only* for reflective
revisions *not always*. How come documenting it helps?

>> + }
>>
>
> To avoid a memory leak, we need to destroy the loop's pool here:
>
> svn_pool_destroy(iterpool);
>

Fixed in r28786.

> Null -> NULL
> enough!!. -> sufficient,
>

Fixed in r28784.

>
>> + As *reflected* summary and reflective merge drive can not give
>>
>
> as
>

Fixed in r28784.

>> + int merge_target_len = strlen(merge_b->target);
>> + const char *file_path_relative_to_target =
>> + path + (merge_target_len ? merge_target_len + 1 : 0);
>> + svn_client_diff_summarize_kind_t *summary_kind =
>> + apr_hash_get(merge_b->reflective_rev_affected_paths,
>> + file_path_relative_to_target,
>> + APR_HASH_KEY_STRING);
>>
>
> This code looks redundant with what's above. Use a helper function?
>

Fixed in r28785.
>
>> +
>> + /* Checking for non-Null summary_kind should be enough!!.
>> + As *reflected* summary and reflective merge drive can not give
>> + two different summaries. */
>>
>
> Ditto.
>

Fixed in r28784.

Thanks Dan.

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-01-08 17:13:27 CET

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