On 9/9/05, Ivan Zhakov <chemodax@gmail.com> wrote:
> Philip,
> Because it was my patch, I reply to you.
>
> On 9/9/05, Philip Martin <philip@codematters.co.uk> wrote:
> > dionisos@tigris.org writes:
> >
> > > Author: dionisos
> > > Date: Thu Sep 8 16:57:26 2005
> > > New Revision: 16094
> >
> > > --- trunk/subversion/libsvn_wc/props.h (original)
> > > +++ trunk/subversion/libsvn_wc/props.h Thu Sep 8 16:57:26 2005
> > > @@ -140,6 +140,11 @@
> > > svn_boolean_t recurse,
> > > apr_pool_t *pool);
> > >
> > > +/* Returns TRUE if any of the PROPCHANGES are the "magic" ones that
> > > + might require changing the working file. At present time magic props are
> > > + SVN_PROP_EXECUTABLE, SVN_PROP_KEYWORDS, SVN_PROP_EOL_STYLE,
> > > + SVN_PROP_SPECIAL, SVN_PROP_NEEDS_LOCK. */
> > > +svn_boolean_t svn_wc__is_magic_props_changed (apr_array_header_t *propchanges);
> >
> > I don't think that's a good name for the function, or for the
> > parameter either. While it may happen to be called with a list of
> > changed properties that's just accidental, in reality it's just a list
> > of properties.
> >
> > svn_boolean_t
> > svn_wc__has_magic_property (apr_array_header_t *properties)
> Agreed. I was inattentive and simply copy and paste code. Sorry.
>
> > You also need to document that the array is an array of svn_prop_t *
> > elements.
> Fixed. But correct your: array is array of "svn_prop_t" structures,
> not pointers to svn_prop_t. Absense of my comments caused error when
> Erik tweaked my patch. Sorry.
>
> > Listing the names of the magic properties in the comment looks a bit
> > odd to me: is the caller supposed to care which properties are magic?
> > Obviously it's a maintenance burden.
> Ok. Check patch for fixes.
>
> [[
> Follow up to r16094. Rename function svn_wc__is_magic_props_changed to
> svn_wc__has_magic_property and clarify its comment.
>
> * subversion/libsvn_wc/adm_ops.c:
> (revert_admin_things): Use new function name svn_wc__has_magic_property.
>
> * subversion/libsvn_wc/props.c:
> (svn_wc__is_magic_props_changed): Rename function to
> svn_wc__has_magic_property.
> Rename function argument from propchanges to properties.
>
> * subversion/libsvn_wc/props.h:
> (svn_wc__is_magic_props_changed): Rename function to
> svn_wc__has_magic_property
> and clarify comment.
>
> * subversion/libsvn_wc/update_editor.c:
> (install_file): Use new function name svn_wc__has_magic_property.
> ]]
>
> --
> Ivan Zhakov
>
Sorry, I forgot attach patch. Now with patch attached.
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Sep 9 12:48:02 2005