"S.Ramaswamy" <ramaswamy@collab.net> writes:
> Changelog:
>
> * subversion/libsvn_client/commit.c
> (repos_change_baton_t) : New struct with a single boolean
> member to remember if changes were made to the repository.
>
So, I like the spirit of the change, but I think you took my
suggestions too literally. As it turns out, there is no existing
context baton for import operations. So why not just add
'svn_boolean_t *' parameters to all the places you added
'repos_change_baton_t *' ones in your patch. We don't to create a
context baton just to hold one variable. And if we were thinking,
"Hey, a context baton is easy to grow later should we need to," then
we should call that thing 'struct import_ctx_t' or something not so
tied to the "Did-the-repos-change" question.
> @@ -387,6 +396,9 @@
> directories. If we stop doing that, here is the place to
> change your world. */
> }
> + /* remember that the repository was modified */
> + if (apr_hash_count (dirents))
> + rcb->repos_changed = TRUE;
>
> svn_pool_destroy (subpool);
> return SVN_NO_ERROR;
I think it makes more sense to put this boolean setting line
immediately after the editor->add_directory() calls to which it
corresponds. Yes, that means it will get hit multiple times, but it
will greatly improve code understandability.
> @@ -437,6 +449,7 @@
> apr_array_header_t *ignores;
> apr_array_header_t *batons = NULL;
> const char *edit_path = "";
> + repos_change_baton_t *rcb = apr_pcalloc (pool, sizeof (*rcb));;
Oops. Double-semicolon.
> Index: subversion/tests/clients/cmdline/basic_tests.py
> ===================================================================
> --- subversion/tests/clients/cmdline/basic_tests.py (revision 14060)
> +++ subversion/tests/clients/cmdline/basic_tests.py (working copy)
> @@ -1632,7 +1632,34 @@
> else:
> raise svntest.Failure
>
> +#----------------------------------------------------------------------
> +def basic_empty_revision(sbox):
> + "verify that an empty revision cannot be created"
This function name and description are entirely too vague. As we know
from experience, 'svn import' isn't the only route by which folks
have, in the past, been able to generate an empty commit. Suggest
moving this test to a new 'import_tests.py' (I'm really surprised we
don't have one of these already!), perhaps as
import_avoid_empty_revision(), and with a more focused description.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 14 16:05:02 2005