Re: [PATCH]: notification progress improvement/fixes (issue #901)

David Anderson wrote:

> Stefan Küng wrote:
>
>> + * In order to avoid backward compatibility problems, clients should
>> use
>> + * svn_ra_create_callbacks() to allocate and initialize this structure
>> + * instead of doing so themselves.
>
>
> Don't give a choice: "In order to ease future compatibility, clients
> must use svn_ra_create_callbacks() to allocate and initialize this
> structure."

Don't even explain why. Just say, "Clients must use
svn_ra_create_callbacks() to allocate and initialize this structure."

>> + neonprogress_baton_t * neonprogress_baton = (neonprogress_baton_t
>> *)baton;
>
>
> You can make this pointer const, to be nicely strict. Also, no space
> between the '*' and the variable name, to remain consistent with the
> surrounding code.

And let's please remove the unnecessary cast from void*. I don't care if
VS.NET gripes about them at warning level 4 -- which is useless for
normal code, anyway -- no-op casts clutter up the code for no benefit.

> - Add a dependancy on libsvn_ra to svnserve in build.conf, otherwise
> it won't build because of an undefined reference to the constructor.

Uh? Where does this dependency come from? I don't see either svnserve or
libsvn_ra_svn using this constructor anywhere.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 31 03:44:31 2005