On Thu, 2005-11-17 at 02:01 +0000, Julian Foad wrote:
> Daniel Berlin wrote:
> >
>
> It looks like Philip has reviewed the code. That's no excuse for me not to
> review it too but I'm tired so I hope you don't mind me just pointing out some
> trivia.
>
> > [[[
> > Add new function to get filenames in a directory without extra stat
> > information, and ues it during revision reporting to avoid extra stat
>
> "ues" -> "use"
I'm kinda ashamed to admit that i use flyspell-prog mode to avoid
spelling errors in comments, but i still get them wrong in log messages
anyway :)
>
> > calls on some filesystems.
> >
> > * subversion/include/svn_io.h
> > (svn_io_get_dir_filenames): New function.
>
> Er... and an implementation for it? :-) Of course there is.
Oh yeah, i forgot subversion/libsvn_subr/io.c
>
> > * subversion/libsvn_wc/adm_crawler.c
> > (report_revisions): Use svn_io_get_dir_filenames, and don't do
> > error checking here.
> >
> > * subversion/libsvn_wc/lock.c
> > (svn_wc_adm_retrieve): Do better error message reporting here.
> > ]]]
>
> > Index: subversion/include/svn_io.h
> > ===================================================================
> > --- subversion/include/svn_io.h (revision 17369)
> > +++ subversion/include/svn_io.h (working copy)
> > @@ -677,6 +677,18 @@ svn_error_t *svn_io_remove_file (const c
> > /** Recursively remove directory @a path. @a path is utf8-encoded. */
> > svn_error_t *svn_io_remove_dir (const char *path, apr_pool_t *pool);
> >
> > +/** Read all of the disk entries in directory @a path, a utf8-encoded
> > + * path. Set @a *dirents to a hash mapping dirent names (<tt>char *</tt>) to
> > + * dirent names, allocated in @a pool.
>
> Mapping dirent names to dirent names? It's a pity a hash can't store NULL as
> the value, but there is probably no reason to advertise duplicate data, so can
> you say "The values are undefined"?
Sure.
>
> I know this is the lowest of trivia, but please could you try to preserve the
> amount of white space except when you intentionally change it. Here, this
> block deletion leaves a blank line after an opening brace; ...
Fixed.
>
> > Index: subversion/libsvn_wc/lock.c
> > ===================================================================
> > --- subversion/libsvn_wc/lock.c (revision 17369)
> > +++ subversion/libsvn_wc/lock.c (working copy)
> > @@ -675,10 +675,89 @@ svn_wc_adm_retrieve (svn_wc_adm_access_t
> > generally makes the calling code simpler as it doesn't need to check
> > for NULL batons. */
> > if (! *adm_access)
> > - return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> > - _("Working copy '%s' is missing or not locked"),
> > - svn_path_local_style (path, pool));
> > + {
> > + const char *wcpath;
> > + const svn_wc_entry_t *subdir_entry;
> > + svn_node_kind_t wckind;
> > + svn_node_kind_t kind;
> > + svn_error_t *err;
> > +
> > + err = svn_wc_entry (&subdir_entry, path, associated, TRUE, pool);
> [...]
> > + /* If all else fails, return our useless generic error. */
> > + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> > + _("Working copy '%s' is not locked"),
> > + svn_path_local_style (path, pool));
> >
> > +
> > + }
> > return SVN_NO_ERROR;
>
> ... and this adds two blank lines before that closing brace and removes the one
> that was before the "return". Let's keep it fanatically tidy!
Okay.
>
> Thanks in advance for not shouting at me. :-)
>
> - Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 17 15:58:09 2005