Re: svn commit: r27982 - in branches/issue-2897/subversion: include include/private libsvn_client libsvn_fs libsvn_fs_base libsvn_fs_fs libsvn_fs_util libsvn_ra libsvn_ra_local libsvn_ra_neon libsvn_ra_serf libsvn_ra_svn libsvn_repos mod_dav_svn mod_

Hi Dave,

> It looks like you assume below that this is either SQLITE_ROW or
> SQLITE_DONE? You should explicitly check for this, and if it isn't,
> use svn_fs__sqlite_stmt_error to extract and return the proper error
> from the statement (which has to happen *before* the reset call).
>
>

Fixed in r28024.

> Ditto with this call. (Also, this block makes allocations in a loop,
> so you might consider an iterpool, though it might be overkill. Maybe
> just a subpool for the whole block.)
>
>

Fixed in r28024.

> Put paths in 'quotes' in error messages. (Helpful if it's empty, say.)
>
>

Fixed in r28024.

> I do think you need to differentiate between SQLITE_DONE here (which
> means your DB doesn't contain some data that should be there according
> to svn) and other sqlite errors (which means that there was some
> problem doing the lookup).
>
>

Fixed in r28024.

> When you merge this to trunk, do make sure to send mail pointing out
> that this will break all existing trunk repos.
>
>

Sure will send a mail.

>> /* USER_VERSION 2 */
>> "CREATE TABLE node_origins (node_id TEXT NOT NULL, node_rev_id TEXT NOT "
>>
>> Modified: branches/issue-2897/subversion/tests/cmdline/merge_tests.py
>> URL: http://svn.collab.net/viewvc/svn/branches/issue-2897/subversion/tests/cmdline/merge_tests.py?pathrev=27982&r1=27981&r2=27982
>> ==============================================================================
>> --- branches/issue-2897/subversion/tests/cmdline/merge_tests.py (original)
>> +++ branches/issue-2897/subversion/tests/cmdline/merge_tests.py Thu Nov 22 11:35:07 2007
>> @@ -9460,7 +9460,7 @@
>> detect_copy_src_for_target_with_multiple_ancestors,
>> prop_add_to_child_with_mergeinfo,
>> diff_repos_does_not_update_mergeinfo,
>> - XFail(avoid_reflected_revs),
>> + avoid_reflected_revs,
>>
>
> That's the nicest bit, isn't it?
>
>

Yes :).

> Also, you added a new ra_svn command, so don't forget to update
> libsvn_ra_svn/protocol (and equivalents for other layers).
>
> --dave
>
>

Yes I remember this, I need to do the same for mod_dav_svn also.

Thanks for your review.

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 26 14:31:23 2007