On Fri, Jun 30, 2006 at 06:28:22AM -0700, sussman@tigris.org wrote:
> Merge changelist-feature branch to trunk.
Wheeeeeeeeeeeeeeeeeeee! Drive-by review follows:
> --- trunk/subversion/include/svn_client.h (original)
> +++ trunk/subversion/include/svn_client.h Fri Jun 30 06:28:20 2006
> @@ -1077,6 +1081,22 @@
> * @since New in 1.3.
New in 1.5.
> */
> svn_error_t *
> +svn_client_commit4(svn_commit_info_t **commit_info_p,
> + const apr_array_header_t *targets,
> + svn_boolean_t recurse,
> + svn_boolean_t keep_locks,
> + const char *changelist_name,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +/** Similar to svn_client_commit4(), but always passes NULL @a
> + * changelist_name.
> + *
> + * @deprecated Provided for backward compatibility with the 1.3 API.
1.4 API.
> + *
> + * @since New in 1.3.
> + */
> +svn_error_t *
> svn_client_commit3(svn_commit_info_t **commit_info_p,
> const apr_array_header_t *targets,
> svn_boolean_t recurse,
> @@ -2455,6 +2475,44 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
>
> +
> +/**
> + * Associate @a path with changelist @a changelist. If CLEAR is set,
> + * then ignore @a changelist and deassociate any existing changelist
> + * from @a path.
CLEAR should be in Doxygen style. But why have a separate variable at
all - why not just take a NULL changelist to be a 'clear' operation?
(I'm not disagreeing that the UI shouldn't use 'svn changelist --clear',
but the API has no need to follow that pattern).
This comment should make it clear what happens if you try to associate
a changelist with a path that's already associated with a changelist -
is it additive, or does it replace the changelist associated with it?
> + *
> + * Note: this metadata is purely a client-side "bookkeeping"
> + * convenience, and is entirely managed by the working copy.
> + *
> + * @since New in 1.5.
> + */
> +svn_error_t *
> +svn_client_changelist(const char *path,
I can't say I'm too happy about the name - 'changelist' isn't a verb,
the UI command notwithstanding.
We have 'retrieve_changelist' elsewhere - why not something like
set_changelist for associating paths? (Oh, and how _do_ you find out
programmatically what changelist (changelists?) are associated with a
given path - ah, svn_client_info?)
Do we need a new version of svn_client_info() that guarantees to call
the receiver with a version of the svn_info_t structure that includes
'changelist'? Or do we assume that if the caller wants to access that
member, they should be clever enough to check that the current runtime
library version is >= 1.5? (via, I guess, something like
svn_subr_version()).
> --- trunk/subversion/include/svn_wc.h (original)
> +++ trunk/subversion/include/svn_wc.h Fri Jun 30 06:28:20 2006
> @@ -3461,6 +3468,24 @@
> void *cancel_baton,
> apr_pool_t *pool);
>
> +
> +/**
> + * Associate @a path with changelist @a changelist by setting the
> + * 'changelist' attribute in its entry. If @a clear is set, then
> + * ignore @a changelist and remove any 'changelist' attribute in @a
> + * path's entry.
> + *
> + * Note: this metadata is purely a client-side "bookkeeping"
> + * convenience, and is entirely managed by the working copy.
> + *
> + * @since New in 1.5.
> + */
> +svn_error_t *
> +svn_wc_changelist(const char *path,
> + const char *changelist,
> + svn_boolean_t clear,
Similar question as to why we don't allow a NULL changelist to indicate
a disassociation.
> svn_error_t *
> +svn_wc__loggy_delete_changelist(svn_stringbuf_t **log_accum,
> + svn_wc_adm_access_t *adm_access,
> + const char *path,
> + apr_pool_t *pool)
Yeah, we really should have some way to clear wc attributes (locks,
changelists) without having to create separate log operations for each.
> --- trunk/subversion/svn/main.c (original)
> +++ trunk/subversion/svn/main.c Fri Jun 30 06:28:20 2006
> @@ -1169,6 +1180,11 @@
> case svn_cl__summarize:
> opt_state.summarize = TRUE;
> break;
> + case svn_cl__clear_opt:
> + opt_state.clear = TRUE;
break;
> + case svn_cl__changelist_opt:
> + opt_state.changelist = apr_pstrdup(pool, opt_arg);
> + break;
> default:
> /* Hmmm. Perhaps this would be a good place to squirrel away
> opts that commands like svn diff might need. Hmmm indeed. */
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 30 18:37:04 2006