You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2010/10/27 10:34:16 UTC
Re: svn commit: r1027853 -
/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
On Wed, Oct 27, 2010 at 08:28:52AM -0000, julianfoad@apache.org wrote:
> Author: julianfoad
> Date: Wed Oct 27 08:28:51 2010
> New Revision: 1027853
>
> URL: http://svn.apache.org/viewvc?rev=1027853&view=rev
> Log:
> * subversion/tests/libsvn_wc/op-depth-test.c
> (wc_wc_copies): Don't fill in part of the baton state here, but ...
> (test_wc_wc_copies, test_reverts): ... fill in all fields in the callers.
>
> Modified:
> subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1027853&r1=1027852&r2=1027853&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Wed Oct 27 08:28:51 2010
> @@ -284,6 +284,34 @@ compare_nodes_rows(const void *key, apr_
> nodes_row_t *expected = apr_hash_get(b->expected_hash, key, klen);
> nodes_row_t *actual = apr_hash_get(b->actual_hash, key, klen);
>
> +#if 1
What's the point of the #if 1 ?
> + /* If the ACTUAL row has field values that should have been elided
> + * (because they match the parent row), then do so now. We want to ignore
> + * any such lack of elision, for the purposes of these tests, because the
> + * method of copying in use (at the time this tweak is introduced) does
> + * calculate these values itself, it simply copies from the source rows. */
> + {
> + const char *parent_relpath, *name, *parent_key;
> + nodes_row_t *parent_actual;
> +
> + svn_relpath_split(&parent_relpath, &name, actual->local_relpath,
> + b->scratch_pool);
> + parent_key = apr_psprintf(b->scratch_pool, "%d %s",
> + actual->op_depth, parent_relpath);
> + parent_actual = apr_hash_get(b->actual_hash, parent_key,
> + APR_HASH_KEY_STRING);
> + if (parent_actual
> + && strcmp(actual->repo_relpath,
> + svn_relpath_join(parent_actual->repo_relpath, name,
> + b->scratch_pool)) == 0
> + && actual->repo_revnum == parent_actual->repo_revnum)
> + {
> + actual->repo_relpath = NULL;
> + actual->repo_revnum = SVN_INVALID_REVNUM;
> + }
> + }
> +#endif
> +
> if (! expected)
> {
> b->errors = svn_error_createf(
Re: svn commit: r1027853 -
/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-10-27 at 12:34 +0200, Stefan Sperling wrote:
> On Wed, Oct 27, 2010 at 08:28:52AM -0000, julianfoad@apache.org wrote:
> > Author: julianfoad
> > Date: Wed Oct 27 08:28:51 2010
> > New Revision: 1027853
> >
> > URL: http://svn.apache.org/viewvc?rev=1027853&view=rev
> > Log:
> > * subversion/tests/libsvn_wc/op-depth-test.c
> > (wc_wc_copies): Don't fill in part of the baton state here, but ...
> > (test_wc_wc_copies, test_reverts): ... fill in all fields in the callers.
> >
> > Modified:
> > subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
> >
> > Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
> > URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1027853&r1=1027852&r2=1027853&view=diff
> > ==============================================================================
> > --- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
> > +++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Wed Oct 27 08:28:51 2010
> > @@ -284,6 +284,34 @@ compare_nodes_rows(const void *key, apr_
> > nodes_row_t *expected = apr_hash_get(b->expected_hash, key, klen);
> > nodes_row_t *actual = apr_hash_get(b->actual_hash, key, klen);
> >
> > +#if 1
>
> What's the point of the #if 1 ?
D'oh, I knew that was asking for it.
It's an indication that the programmer isn't really sure what he's
doing. :-) The programmer (that is: I) thinks somebody might want to
disable this code some time, and wants to indicate the extent of the
block that can be omitted.
But it's ugly without an explanatory comment. I'll remove it or comment
it.
Thanks.
- Julian
> > + /* If the ACTUAL row has field values that should have been elided
> > + * (because they match the parent row), then do so now. We want to ignore
> > + * any such lack of elision, for the purposes of these tests, because the
> > + * method of copying in use (at the time this tweak is introduced) does
> > + * calculate these values itself, it simply copies from the source rows. */
> > + {
> > + const char *parent_relpath, *name, *parent_key;
> > + nodes_row_t *parent_actual;
> > +
> > + svn_relpath_split(&parent_relpath, &name, actual->local_relpath,
> > + b->scratch_pool);
> > + parent_key = apr_psprintf(b->scratch_pool, "%d %s",
> > + actual->op_depth, parent_relpath);
> > + parent_actual = apr_hash_get(b->actual_hash, parent_key,
> > + APR_HASH_KEY_STRING);
> > + if (parent_actual
> > + && strcmp(actual->repo_relpath,
> > + svn_relpath_join(parent_actual->repo_relpath, name,
> > + b->scratch_pool)) == 0
> > + && actual->repo_revnum == parent_actual->repo_revnum)
> > + {
> > + actual->repo_relpath = NULL;
> > + actual->repo_revnum = SVN_INVALID_REVNUM;
> > + }
> > + }
> > +#endif
> > +
> > if (! expected)
> > {
> > b->errors = svn_error_createf(