Thanks for the review Philip. My Subversion style is apparently a
little rusty. Things should be fixed up in r11610.
-Josh
Philip Martin wrote:
> jpieper@tigris.org writes:
[... code ... ]
> > + /* Verify that we pass at most one working copy path. */
> > + target = APR_ARRAY_IDX (targets, 0, const char *);
>
> There's an identical line inside the else if(), perhaps it should be
> moved before the if().
>
> > +
> > + if (! svn_path_is_url (target) )
> > + {
> > + if (targets->nelts > 1)
> > + return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> > + _("When specifying working copy paths, only "
> > + "one target may be given"));
> > + } else {
>
> "} else {" is usually written on three lines in the Subversion code.
>
> > + /* Check to make sure there are no other URLs. */
> > + for (i = 1; i < targets->nelts; i++) {
>
> "for () {" is usually written on two lines.
>
> > + target = APR_ARRAY_IDX (targets, 1, const char *);
>
> Huh? In that APR_ARRAY_IDX should 1 be i?
>
> > +
> > + if (svn_path_is_url (target))
> > + return svn_error_create (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> > + _("Only relative paths can be specified "
> > + "after a URL"));
> > + }
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Oct 24 15:38:00 2004