Hi Daniel,
Daniel Shahaf writes:
> > - * @since New in 1.1.
> > + * @since New in 2.0.
>
> s/2.0/1.7/
Fixed.
> > +/**
> > + * Similar to svn_repos_parse_dumpstream3(), but is dumpfile version
> > + * agnostic.
> > + *
>
> Perhaps say: "with @a version_number always set to -1" ?
Fixed, along with Hyrum's suggestions.
> > + /* Error out if version doesn't match with the provided version_number */
> > + if (version_number != -1 && version_number != version)
>
> svnrdump needs an inequality check here. But 'svnadmin load', for
> example, needs an "is at most" check. So I don't think this is generic
> enough: it would be better to provide something that 'svnadmin load' can
> use too.
>
> So, two options:
>
> * Repeat the parse_format_version() trick (from load.c) in svnrdump.
It's actually not possible. If I read one line of the dumpstream in
load_editor.c and then call svn_repos_parse_dumpstream2, the function
won't have the version number information because I've already read
that out from the dumpstream, and I can't rewind the stream. Note that
as you already pointed out on IRC, parse_format_version imposes a
maximum already (load.c:647).
> * In the API, PARSE_FNS could grow a dumpfile_version_record() callback
> (like it has a uuid_record()) callback. The caller can implement
> whatever check they want in that callback. (possibly with a special
> error code that svn_repos_parse_dumpstreamN() recognizes; compare
> usage of SVN_ERR_CANCELLED)
Excellent idea; too much work for one patch though- I'll add a TODO
comment about this and re-post the patch.
-- Ram
Received on 2010-08-18 23:11:20 CEST