Re: [REVIEW]: ISO 8601 Relaxation With Err Msg's

I don't really support relaxing the date formats (for reasons I can't
necessarily articulate, other than that we'd be departing from the ISO
date format spec), but I also found a bunch of things wrong with this
patch.

First and foremost, you combined two changes which can easily be
separated: relaxing the date formats, and changing the handling of
date-parsing errors.

You also used C++-style comments, and generally didn't adhere to our
formatting rules or even use internally consistent indentation.

In svn_opt.h:

-int svn_opt_parse_revision (svn_opt_revision_t *start_revision,
+svn_error_t *
+ svn_opt_parse_revision (svn_opt_revision_t *start_revision,

You can't change the signature of a function in the Subversion API. You
have to rev it (look in our headers for other examples of how interfaces
have been revved).

In opt.c:

+ @a ret_error allows us to pass up error data without needing to
+ completely refactor the rest of this function.

This is design through accretion. We don't do that.

+ // This error message is overridden by the one in the client.
+ return svn_error_createf (SVN_ERR_BAD_DATE, NULL,
+ "Date not in ISO-8601 or GNU format");

Why?

+ if (str_err == SVN_NO_ERROR)
+ return svn_cmdline_handle_exit_error (err, pool, "svn:
");

You're comparing a char * to a constant intended to be used with
svn_error_t *s. This block in general looks confused.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 20 23:06:20 2005