Re: svn commit: r27424 - in trunk/subversion: libsvn_ra_serf tests/cmdline

[Lieven Govaerts <lg...@mobsol.be>]

Karl Fogel wrote:
>> +
>> +static svn_error_t *
>> +get_version_url(svn_ra_serf__session_t *session, 
>> +                svn_ra_serf__connection_t *conn,
>> +                const char *name, 
>> +                svn_revnum_t base_revision,
>> +                const char *parent_vsn_url,
>> +                const char **checked_in_url,
>> +                apr_pool_t *pool)
>> +{
>> +
>> + [...]
> 
> Heh.  Because get_version_url() was moved within the file, as well as
> modified, it's hard to see the diff.  I'm not saying you should have
> done things differently -- it had to be moved -- it's just too bad.
> 
I noticed. Don't really know how to prevent this though, do you have any
suggestions? Separating in two commits would work, but that seems
overkill for what's basically a 'unified diff readability' issue.

> Unfortunately, ra_serf is not great about documenting its internal
> static functions, but when we're exploding a single parameter ('dir')
> to a bunch of parameters, it would be a real help to document the
> rewritten function...

Agreed.

> I tried it in r27515, please let me know if it reads right to you.

Thanks, looks good. In r27550 I tried to structure it a bit better,
don't know if I succeeded.

Lieven

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r27424 - in trunk/subversion: libsvn_ra_serf tests/cmdline

[Karl Fogel <kf...@red-bean.com>]

Lieven Govaerts <lg...@mobsol.be> writes:
> I noticed. Don't really know how to prevent this though, do you have any
> suggestions? Separating in two commits would work, but that seems
> overkill for what's basically a 'unified diff readability' issue.

Nah, I think the way you did it was fine.  I was just whining.

> Thanks, looks good. In r27550 I tried to structure it a bit better,
> don't know if I succeeded.

+1 on the changes -- the new version is actually more correct, in some
subtle ways, I think.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org