[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r979011 - in /subversion/branches/svnrload: build.conf subversion/svnrload/ subversion/svnrload/load_editor.c subversion/svnrload/parse_dumpstream.c subversion/svnrload/svnrload.c

From: Ramkumar Ramachandra <artagnon_at_gmail.com>
Date: Sun, 25 Jul 2010 18:44:33 +0530

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

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.