Hi Daniel,
Daniel Shahaf writes:
> Please don't mix whitespace/indentation changes and semantic changes in
> the same commit.
>
> (I'm sure HACKING says that --- doesn't it?)
Afaik, no. It says is "A patch submission should contain one logical
change; please don't mix N unrelated changes in one submission". Okay,
will remember this next time.
> How am I supposed to review the changes to this function?
Please don't. I forgot to mark the commit "don't review".
> > + struct parser_baton_t *parser_baton;
> > +
> > + if (svn_cmdline_init ("parse_dumpstream", stderr) != EXIT_SUCCESS)
> > + return EXIT_FAILURE;
> > + pool = svn_pool_create(NULL);
> > +
> > + parser = apr_pcalloc(pool, sizeof(*parser));
> > +
> > + /* parser->new_revision_record = new_revision_record; */
> > + /* parser->new_node_record = new_node_record; */
> > + /* parser->uuid_record = uuid_record; */
> > + /* parser->set_revision_property = set_revision_property; */
> > + /* parser->set_node_property = set_node_property; */
> > + /* parser->remove_node_props = remove_node_props; */
> > + /* parser->set_fulltext = set_fulltext; */
> > + /* parser->close_node = close_node; */
> > + /* parser->close_revision = close_revision; */
> > + /* parser->delete_node_property = delete_node_property; */
> > + /* parser->apply_textdelta = apply_textdelta; */
> > +
>
> All this are "to be done", I guess?
Yeah.
> > +/* const char *url = NULL; */
> > +/* char *revision_cut = NULL; */
> > +/* svn_revnum_t start_revision = svn_opt_revision_unspecified; */
> > +/* svn_revnum_t end_revision = svn_opt_revision_unspecified; */
> > +/* svn_revnum_t latest_revision = svn_opt_revision_unspecified; */
> > +/* svn_boolean_t quiet = FALSE; */
> > +/* apr_pool_t *pool = NULL; */
> > +/* svn_ra_session_t *session = NULL; */
> > +/* const char *config_dir = NULL; */
> > +/* const char *username = NULL; */
> > +/* const char *password = NULL; */
> > +/* svn_boolean_t no_auth_cache = FALSE; */
> > +/* svn_boolean_t non_interactive = FALSE; */
> > +/* apr_getopt_t *os; */
> > +
>
> Why not "#if 0" the block?
Good suggestion. Will do that next time.
> > +/* SVN_INT_ERR(svn_cmdline_fprintf(stderr, pool, */
> > +/* "LOWER cannot be greater " */
> > +/* "than UPPER.\n")); */
>
> Unrelated, but again the error message says LOWER and UPPER. Could you
> rephrase it in a way that uses the semantics of the arguments rather
> than names in the help syntax?
> e.g., I'm sure we already have such a message somewhere... (/me opens
> fr.po) Here it is:
>
> #: ../libsvn_repos/dump.c:1048 ../libsvn_repos/dump.c:1290
> #, c-format
> msgid "Start revision %ld is greater than end revision %ld"
> msgstr "La révision de début %ld est plus grande que celle de fin %ld"
Will do. Thanks.
-- Ram
Received on 2010-07-25 15:17:04 CEST