> Madan US wrote:
>> [[[
>> Partial Fix for Issue #443: post-commit hook script (error) output
>> lost
>>
>
> Please begin the log message with a description of what the patch does
> (and why, if that's not obvious).
Above is a snip from the description of the issue at
subversion.tigris.org... Anyways, I would put in more info hereon.
>
>
>> Index: subversion/include/svn_client.h
>> ===================================================================
>> --- subversion/include/svn_client.h (revision 14776)
>> +++ subversion/include/svn_client.h (working copy)
> [...]
>> @@ -653,60 +679,90 @@
>> svn_client_ctx_t *ctx,
>> apr_pool_t *pool);
>>
>> -/** Create a directory, either in a repository or a working copy. - *
>> - * If @a paths contains URLs, use the authentication baton in @a ctx
>> - * and @a message to immediately attempt to commit the creation of
>> the - * directories in @a paths in the repository. If the commit
>> succeeds, - * allocate (in @a pool) and populate @a *commit_info.
> [...]
>> +/** @since New in 1.3.
>> + * Create a directory, either in a repository or a working copy. +
>> *
>> + * If @a paths contains URLs, use the authentication baton in @a ctx
>> + * and @a message to immediately attempt to commit the creation of
>> the + * directories in @a paths in the repository. If the commit
>> succeeds, + * allocate (in @a pool) and populate @a *commit_info.
> [...]
>
> Please don't change the indentation. This is part of the reason why
> this patch is so big.
mmm... mind of a programmer.... ;-).... doesnt accept something that doesnt
fit his visualization ....;)
>
>
>> Index: subversion/libsvn_client/copy.c
>> ===================================================================
>> --- subversion/libsvn_client/copy.c (revision 14776)
>> +++ subversion/libsvn_client/copy.c (working copy)
>> @@ -269,7 +269,235 @@
>> return SVN_NO_ERROR;
>> }
>>
>> +/** @since New in 1.3.
>> + *
>> + * Now uses svn_client_commit_info2_t which is post-commit
>> + * hook's stderr aware.
>> + */
>> +static svn_error_t *
>> +repos_to_repos_copy2 (svn_client_commit_info2_t **commit_info,
>> + const char *src_url,
>> + const svn_opt_revision_t *src_revision,
>> + const char *dst_url,
>> + svn_client_ctx_t *ctx,
>> + svn_boolean_t is_move,
>> + apr_pool_t *pool)
>> +{
> [about 200 lines]
>> +}
>
> I don't think it's acceptable to duplicate several large functions like
> this one just to change two lines in them. You will have to find a
> way to factor out the common parts.
True... I ran out of ideas here... could you pl. suggest an alternate
implementation for this one...
>
>> +/** @deprecated Provided for backward compatibility
>> + *
>> + * Similar to repos_to_repos_copy2, but uses
>> svn_client_commit_info_t + * for @a commit_info type
>> + */
>> static svn_error_t *
>> repos_to_repos_copy (svn_client_commit_info_t **commit_info,
>> const char *src_url,
>> @@ -579,6 +807,11 @@
>>
>>
>>
>> +/** @deprecated Provided for backward compatibility.
>> + *
>> + * Similar to wc_to_repos_copy but uses svn_client_commit_info_t +
>> * for the @a commit_info parameter.
>> + */
>> static svn_error_t *
>> wc_to_repos_copy (svn_client_commit_info_t **commit_info,
>> const char *src_path,
>
> These comments are inappropriate. A static function isn't deprecated,
> it's either used or deleted; in this case is it used (by a deprecated
> function). We only use Doxygen mark-up in public API headers.
Yes, so I just leave this uncommented??? How do I say that this has two
versions and that the old version should be removed sometime in future?? Pl.
advise.
> "Similar to wc_to_repos_copy"? ... this is "wc_to_repos_copy".
oops... typo should have been wc_to_repos_copy2.
>
>
> This message was a bit big for the mailing list at 161 KB. When you
> have a big patch, please consider making it smaller or posting a URL
> to it instead,
I didnt think of this... would do this henceforth. Sorry for that.
> or take it as a sign that there may be something wrong
> with the patch.
Something wrong with the patch??? IMHO, I dont think so... the patch is so
big because of deprecating functios thorughout the code ( from one end of
the client to the other end of the server ) over all three protocols.
Pl. correct me if I'm wrong.
>
> I haven't reviewed your implementation. I'm glad you're attacking the
> problem, though.
Cool, and am happy this patch is getting noticed inspite of the size.
Thanks, Julian.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun May 22 08:11:46 2005