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

Re: [PATCH] Issue #1295 (take 2, with log)

From: Branko Čibej <brane_at_xbc.nu>
Date: 2003-05-09 09:32:24 CEST

Mark Grosberg wrote:

>On 9 May 2003 kfogel@collab.net wrote:
>
>
>
>>Always include the full log message with every post of a patch. You
>>can also give metadata like "Repost number 3 of the blah blah patch"
>>
>>
>
>Yeah, I really meant to do that. I was fixing my A/C while doing this
>patch and I think the PVC glue fumes got to me. ;-) Down here you can't
>live an hour without the A/C.
>
>
>
>>So, please repost this exact same patch, together with its log message
>>:-).
>>
>>
>
>Done. And I also address some formatting fubar.
>
This patch looks much, much better to me than the first one. Very nice!

[snip]

> /** The commit candidate structure. */
>Index: libsvn_client/commit_util.c
>===================================================================
>--- libsvn_client/commit_util.c (revision 5857)
>+++ libsvn_client/commit_util.c (working copy)
>@@ -730,6 +730,14 @@
> = *((const svn_client_commit_item_t * const *) a);
> const svn_client_commit_item_t *item2
> = *((const svn_client_commit_item_t * const *) b);
>+
>+ if ((item1->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED)
>+ && (!(item2->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED)))
>+ return 1;
>+ if ((!(item1->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED))
>+ && (item2->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED))
>+ return -1;
>+
> return svn_path_compare_paths (item1->url, item2->url);
> }
>
Heh.

    int cmp = ((item1->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED)
               - (item2->state_flags & SVN_CLIENT_COMMIT_ITEM_UNMARKED));
    if (cmp == 0)
      cmp = svn_path_compare_paths (item1->url, item2->url);
    return cmp;

[snip]

>+/* Parse the edited comitted message and store the named paths
>+ in the hash. */
>+static void
>+process_comitted_list (apr_hash_t *seen,
>+ char *commited_list,
>+ apr_pool_t *pool)
>

The docstring for this function should explain exactly what kind of
input it expects, what it'll parse and when it'll fail. I'm also a bit
worried about its silently doing nothing if it encounters something it
can't parse; I think that's a potential source of many surprises and
"error" reports; we want to avoid that, so giving a bit of though to the
failure cases would be good here.

And we really want tests for this feature (in commit-tests.py).
Modifying the commit list in a message passed in with -F should work, right?

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 9 09:35:18 2003

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.