Re: [PATCH] New dumpstream parser to check version number

Hi Hyrum,

Hyrum K. Wright writes:
> > + * If @a version_number is -1, it is ignored and the dumpstream is
> > + * parsed without this information. If not -1, the function checks the
>
> We try to avoid magic numbers. Is there a #define or constant
> somewhere that you could use? If not, I'd add a #define for
> UNKNOWN_VERSION or something similar (look at other #define's in the
> header files).

Right, done.

> > - * @since New in 1.1.
> > + * @since New in 2.0.
>
> Uh, we haven't (not do we have an immediate plans to) released 2.0 yet. :)

Right. I seem to have forgotten about 1.7-1.9 :p

> > +/**
> > + * Similar to svn_repos_parse_dumpstream3(), but is dumpfile version
> > + * agnostic.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.6 API.
>
> Should maintain the @since tag from before.

Ok.

> > +  /* Error out if version doesn't match with the provided version_number */
> > +  if (version_number != -1 && version_number != version)
>
> Another magic number.

Fixed.

> > -  SVN_ERR(svn_repos_parse_dumpstream2(stream, parser, parse_baton,
> > -                                      NULL, NULL, pool));
> > +  SVN_ERR(svn_repos_parse_dumpstream3(stream, parser, parse_baton,
> > +                                      NULL, NULL, 3, pool));
>
> Another magic number.

Fixed.

-- Ram
Received on 2010-08-18 22:53:43 CEST