D3557: extensions: new closehead module for closing arbitrary heads

[pulkit (Pulkit Goyal)]

pulkit added a comment.


  Thanks for taking this out into a separate extension. Code looks mostly good to me. It feels to me that we need some more tests:
  
  - passing multiple revisions when some are head and some not
  - closing a head which has secret phase
  - checking for bookmark movement
  
  I know the behavior in couple of the above points but having tests will help us in future when we change some behavior, like showing warning for revs which are not heads and closing others.
  
  Also, wherever we pass revisions, we have support for `--rev` flag too. Can you add that too?

INLINE COMMENTS

> closehead.py:33
> +
> + at command('close|close-head|close-heads', commitopts + commitopts2,
> +    _('[OPTION]... [REV]...'), inferrepo=True)

close is too generic, I don't feel confident enough to have this as a command name. Can we drop that for now if you don't feel to strong?

> closehead.py:53
> +        tr.close()
> +
> +    if not revs:

please add an `opts = pycompat.byteskwargs(opts)` call here for python 3 support.

> closehead.py:69
> +    if not message:
> +        raise error.Abort(_("No commit message specified with -l or -m"))
> +    extra = { 'close': '1' }

we don't start with uppercase in error messages

> closehead.py:73
> +    wlock = lock = None
> +    try:
> +        wlock = repo.wlock()

nit: how about using context manager?

> closehead.py:79
> +            branch = r.branch()
> +            if branch:
> +                extra['branch'] = branch

I don't think this will ever go the else part.

> test-close-head.t:1
> +  $ hg init
> +  $ hg debugbuilddag '+2*2*3*4'

init a new repo and cd into that. don't work in top level $TESTTMP directory.

> test-close-head.t:43
> +
> +  $ hg init test-empty
> +  $ cd test-empty

same, don't create a new inside existing repo, cd back and then create one.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers
Cc: indygreg, pulkit, mercurial-devel