Re: svn commit: r16094 - trunk/subversion/libsvn_wc

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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org