Re: [PATCH v3] nmbug: Translate to Python

["W. Trevor King" <wking@tremily.us>]

[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]

On Sun, Jul 20, 2014 at 03:59:49PM -0700, W. Trevor King wrote:
> +def pull(repository=None, refspecs=None):
> +    """
> +    Pull (merge) remote repository changes to notmuch.
> +
> +    'pull' is equivalent to 'fetch' followed by 'merge'.  We use the
> +    Git-configured repository for your current branch
> +    (branch.<name>.repository, likely 'origin', and
> +    branch.<name>.merge, likely 'master').
> +    """
> +    _insist_committed()
> +    if refspecs and not repository:
> +        repository = _get_remote()
> +    args = ['pull']
> +    if repository:
> +        args.append(repository)
> +    if refspecs:
> +        args.extend(refspecs)
> +    with _tempfile.TemporaryDirectory(prefix='nmbug-merge.') as workdir:
> +        _git(args=args, additional_env={'GIT_WORK_TREE': workdir}, wait=True)
> +    checkout()

The TemporaryDirectory prefix should probably be 'nmbug-pull.'.
Queued for v4.

> +def log(args=()):
> +    """
> +    A simple wrapper for 'git log'.
> +
> +    After running 'nmbug fetch', you can inspect the changes with
> +    'nmbug log HEAD..@{upstream}'.
> +    """
> +    # we don't want output trapping here, because we want the pager.
> +    args = ['git', '--git-dir', NMBGIT, 'log', '--name-status'] + list(args)
> +    _LOG.debug('exec {args}'.format(args=args))
> +    _os.execvp('git', args)

I don't exec any other commands.  Maybe we want '_git(args=args,
wait=True)' here (with the appropriate args adjustments)?

> +def _diff_index(index, filter):
> +    """Get an {id: {tag, ...}} dict for a given filter.
> +
> +    For example, use 'A' to find added tags, and 'D' to find deleted tags.
> +    """

I'll shift the summary onto the next line here to match the pattern
set by the command functions (e.g. archive()).  They *need* the
summary to be on the line after the opening triple-quote to support
the textwrap.dedent() help used for the argument parser.

There were also a few docstrings missing the trailing period
recommended by PEP 257 [1] (for _hex_quote, get_tags, _read_tree,
fetch, _index_tags, and _unpack_diff_lines).  I'll add those periods
in v4.

Cheers,
Trevor

[1]: http://legacy.python.org/dev/peps/pep-0257/#one-line-docstrings

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]