Re: [Monotone-devel] automate log

[Thomas Keller]

Am 03.07.2010 18:46, schrieb Stephen Leake:
> In branch nvm.stephe, I've added a command 'mtn automate log'. It
> produces the same list of revisions as 'mtn log', but the only output is
> the revision ids, on stdout and the stdio 'm' channel.
> 
> It takes a subset of the 'log' options; all options that control output
> are gone; those that determine what revisions to show remain.
> 
> The front-end running 'mtn autotmate' can retrieve information about
> each revision as needed. This simplifies several things in Emacs DVC.
> 
> See tests/automate_log/__driver__.lua for examples.
> 
> This could be seen as an alternative to 'mtn automate select'. One
> essential difference is that 'automate log' accepts a path restriction,
> which 'automate select' does not. The other 'log' options, particularly
> --last and --next, are also useful.
> 
> Ok to merge to main?

A few objections:

1) in monotone.texi / cmd_diff_log.cc:

  address@hidden address@hidden address@hidden [...]]
  address@hidden [...]] [--no-merges] [--no-files]

  If only revision ids are outputted, the --no-files option is bogus

2) in monotone.texi:

  +These messages are in the 'm' stream in automate stdio.

  All the normal output for every automate command goes to this stream.
  Since we do not mention it anywhere else explicitely, we shouldn't do
  that here either.

3) in cmd_diff_log.cc:

  +void
  +log_print_rev (app_state &      app,
  +               database &       db,
  +               project_t &      project,
  +               revision_id      rid,
  +               revision_t &     rev,
  +               string           date_fmt,
  +               node_restriction mask,
  +               bool             automate,
                  ^^^^

  Please use an enum for this and later uses. Also, because
  log_print_rev is only used twice and the use is determined with this

  if (automate)
    log_print_rev (app, db, project, rid, rev, date_fmt, mask, \
                   automate, output);
  else
    ostringstream out;
    log_print_rev (app, db, project, rid, rev, date_fmt, mask, \
                   automate, out);

  The code could be simplified like this:

  if (target == automate_log)
    out << rid << "\n";
  else
    {
       // the former code which prints everything else ...
    }

4) You have quite a lot whitespace changes / unrelated changes e.g. in
testlib.lua and a couple of others. Some of them seem to be needed for
your current work, but in general please try to keep the patch at a
minimum and commit unrelated things directly to mainline next time.


Thomas.

-- 
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

signature.asc
Description: OpenPGP digital signature