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

Re: svn commit: r19500 - in branches/changelist-feature/subversion: include libsvn_client libsvn_wc svn

From: Ben Collins-Sussman <sussman_at_red-bean.com>
Date: 2006-05-08 22:21:32 CEST

Thanks for the review, Philip. I've fixed most of the typos and
little things you pointed out. Here are a few responses to some
larger items.

> Perhaps mention the encoding, I guess it's UTF8?

This is true of *all* paths in our internal APIs, isn't it? We
shouldn't have to explicitly say this, should we?

>
> Did you consider using two functions, set and clear?

I did, but it seemed simpler to me to have one function to 'diddle a
changelist' association. If people feel strongly, I can certainly
divide it into two separate functions.

> > +
> > + SVN_ERR(svn_wc__entry_modify(adm_access, entry->name, &newentry,
> > + SVN_WC__ENTRY_MODIFY_CHANGELIST,
> > + TRUE, pool));
> > + SVN_ERR(svn_wc_adm_close(adm_access));
> > +
> > + return SVN_NO_ERROR;
>
> I expect we will get requests to allow recursive operations, if so
> then open/modify/write on each item approach might be a bottleneck.
> It's probably best to make it recursive now (or document a strong
> argument that will preempt the need to rev the interface :)

I think I agree in principle, but I don't want to get bogged down
(yet) in implementing a -R switch. Let me add it to my to-do list as
something to do once I get the feature working. :-)

> It's a bit of an odd interface, given:
>
> $ svn changelist change1 foo bar
> $ svn changelist change2 zig zag
>
> I have to give a changelist name here:
>
> $ svn changelist --clear change2 foo zag
>
> but it's more or less redundant. Should that command clear foo from
> the change1 changelist or should it warn that foo is not part of
> change2?

I agree. I think the UI should just be:

  $ svn changelist change1 foo bar # adds a label to foo and bar
  $ svn changelist --clear foo bar baz # removes all changelist
labels from 3 files

In other words, if the --clear flag is present, we assume all targets
are files. If the --clear flag isn't present, we assume the first
target is a changelist name.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon May 8 22:22:07 2006

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