On Mon, 29 Aug 2005, Stefan Küng wrote:
> Peter N. Lundblad wrote:
> > On Sun, 28 Aug 2005 brane@tigris.org wrote:
> >
> >>Log:
> >>Implement a reduced version of issue #901 for DAV connections.
> >>
> >
> > So, one has to look up the issue to know what this patch is doing. One
> > more sentence telling what the change is for would be good.
>
> I can't change that since I don't have commit access. Brane, can you
> please do it?
>
OK. I took that one myself. We normally fix other committers small
commit message mistakes when we see them, I wanted to point it out (mostly
directed at brane) because it is annoying when people just refer to an
issue number out of context.
> >>+/** A collection of callbacks implemented by libsvn_client which allows
> >>+ * an RA layer to "pull" information from the client application, or
> >>+ * possibly store information. libsvn_client passes this vtable to
> >>+ * @c RA->open().
> >>+ *
> >>+ * Each routine takes a @a callback_baton originally provided with the
> >>+ * vtable.
> >>+ *
> >>+ * @deprecated Provided for backward compatibility with the 1.2 API.
> >> */
> >> typedef struct svn_ra_callbacks_t
> >
> >
> > Should change the above docs to just say "Similar to ... except ...". And
> > could we get rid of the duplicate documentation for these fields?
>
> Something like
> /** Similar to svn_ra_callbacks2_t, except that the progress
> * notification function and baton is missing.
> *
> * @deprecated Provided for backward compatibility with the 1.2 API.
> */
>
Yeah, or even
"Similar to @c svn_ra_callbacks2_t, except that @c progress_func and @c
progress_baton are not present". The important difference is that the
members are referred directly by their names instead of inventing a new
term.
> >>+static void
> >>+ra_dav_neonprogress(void * baton, off_t progress, off_t total)
> >>+{
> >>+ /* Important: don't change the svn_ra_callbacks2_t struct here! */
> >
> >
> > So, that's why it is const below? (Point is: why this comment?)
>
> To make sure that if someone changes that code later for whatever
> reason, it's immediately clear what's allowed and what's not.
> Aren't comments there for such reasons too?
>
And as we've seen this comment can be misunderstood, since I just did
so:-) When I read it, I interpreted it like "don't change the content of
that variable" which is exactly what the const keyword tells both the
reader and the compiler. I would instead say that it is important that
this variable remains const and why.
Thanks for tracking this commit on the dev@ list. I'll look at your next
patch.
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 29 20:40:01 2005