On Mon, Apr 18, 2011 at 15:41, <rhuijben_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Mon Apr 18 19:41:26 2011
> @@ -603,6 +603,11 @@ SELECT md5_checksum
> FROM pristine
> WHERE checksum = ?1
>
> +-- STMT_SELECT_PRISTINE_SIZE
> +SELECT size
> +FROM pristine
> +WHERE checksum = ?1 LIMIT 1
checksum is the PRIMARY KEY. Thus, it is unique and will select just
one row. The LIMIT 1 is superfluous, and could even be construed as
misleading ("what? more than one row could get selected?")
>...
> +++ subversion/trunk/subversion/libsvn_wc/workqueue.c Mon Apr 18 19:41:26 2011
> @@ -625,7 +625,7 @@ process_commit_file_install(svn_wc__db_t
> "repair" them if the file is unmodified */
> SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL,
> db, local_abspath,
> - TRUE, FALSE, scratch_pool));
> + FALSE, FALSE, scratch_pool));
I find this block of the 'if' statement to be very poor form. We don't
use the MODIFIED result, so why are we calling this function? Just for
its *side effects*? That sounds rather janky. Why don't we simply grab
the values and shove them into the db. In other words... just like the
true-block of the 'if' statement. Seems that we can just strike the
'if' and unconditionally grab new values to shove into the db with a
direct call to record_fileinfo().
>...
Cheers,
-g
Received on 2011-04-18 23:16:44 CEST