[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Follow-up to r922176 was:Re: svn commit: r922176 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_wc/revision_status.c svnversion/main.c

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Wed, 31 Mar 2010 10:19:10 +0100

On Tue, 2010-03-30, Daniel Näslund wrote:
> Ping! This patch has not been revieved

[...]
> > [[[
> > Follow-up to r922176. Fix that tree changes were not considered when
> > determining if the wc has modifications.
> >
> > * subversion/libsvn_wc/revision_status.c
> > (analyze_status): Determine from status if a path has been added or
> > deleted. Do some optimizations to avoid having to do a text
> > comparison for determining if a wc has modifications.
> >
> > Suggested by: gstein
> > rhuijben
> > Approved by: ?
> > ]]]
>
> > Index: subversion/libsvn_wc/revision_status.c
> > ===================================================================
> > --- subversion/libsvn_wc/revision_status.c (revision 922398)
> > +++ subversion/libsvn_wc/revision_status.c (arbetskopia)
> > @@ -41,7 +41,8 @@
> > };
> >
> > /* An svn_wc__node_found_func_t callback function for analyzing the status
> > - * of nodes */
> > + * of nodes. Optimized to avoid text compares and unneccessary checks of
> > + * already set values. */

Be more specific: Which nodes does it analyze? How does it return the
result? What kinds of status can it report? (A reference to somewhere
else is fine, if the details are already explained somewhere else.)
Then the "Optimised ..." sentence should make more sense.

> > static svn_error_t *
> > analyze_status(const char *local_abspath,
> > void *baton,
> > @@ -50,10 +51,9 @@
> > struct walk_baton *wb = baton;
> > svn_revnum_t changed_rev;
> > svn_revnum_t revision;
> > + svn_revnum_t item_rev;
> > svn_depth_t depth;
> > svn_wc__db_status_t status;
> > - svn_boolean_t wc_root;
> > - svn_boolean_t switched;
> >
> > SVN_ERR(svn_wc__db_read_info(&status, NULL, &revision, NULL,
> > NULL, NULL, &changed_rev,
> > @@ -71,24 +71,36 @@
> > wb->result->sparse_checkout = TRUE;
> > return SVN_NO_ERROR;
> > }
> > + else if (status == svn_wc__db_status_not_present)
> > + {
> > + return SVN_NO_ERROR;
> > + }
> > + else if (status == svn_wc__db_status_added
> > + || status == svn_wc__db_status_obstructed_add
> > + || status == svn_wc__db_status_deleted
> > + || status == svn_wc__db_status_obstructed_delete)
> > + {
> > + wb->result->modified = TRUE;
> > + }

(Minor nit: "else" is redundant after a "return". I don't particularly
mind, but somebody objected to them the other day.)

The rest looks fine.

- Julian
Received on 2010-03-31 11:19:47 CEST

This is an archived mail posted to the Subversion Dev mailing list.