D636: cmdutil: remove the redundant commit during amend

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Thu Sep 7 15:19:29 EDT 2017


martinvonz added a comment.


  I fixed a few nits in flight and queued this. Thanks for cleaning this up!

INLINE COMMENTS

> cmdutil.py:3074-3078
> +        matcher = scmutil.match(wctx, pats, opts)
> +        if (opts.get('addremove')
> +            and scmutil.addremove(repo, matcher, "", opts)):
> +            raise error.Abort(
> +                _("failed to mark all new/missing files as added/removed"))

Looks like this loses the dirstateguard from cmdutil.commit() (and duplicates the message). Perhaps it will be easier to let the caller do the addremove() call and the locking (it already does the locking, so the locking above seems unnecessary). Anyway, this seems fine for now.

> cmdutil.py:3080
> +
> +        filestoamend = set([f for f in wctx.files() if matcher(f)])
> +

nit: No need to make a list and then a set, you should be able to just drop the brackets

> cmdutil.py:3104
> +                    # changes to other files from the old changeset.
> +                    if not path in filestoamend:
> +                        return old.filectx(path)

nit: usually written "path not in filestoamend"

> cmdutil.py:3195-3196
> +        # and modified in the amend to "normal" in the dirstate.
> +        normalfiles = set(
> +            wctx.modified() + wctx.added()).intersection(filestoamend)
> +        for f in normalfiles:

nit: i think we usually write "a & b" instead of "a.intersection(b)"

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D636

To: singhsrb, #hg-reviewers
Cc: martinvonz, mercurial-devel


More information about the Mercurial-devel mailing list