D3827: rebase: no need to store backup during dry-run while aborting

yuja (Yuya Nishihara) phabricator at mercurial-scm.org
Sat Jun 23 23:12:30 EDT 2018


yuja added a comment.


  > - def _prepareabortorcontinue(self, isabort): +    def _prepareabortorcontinue(self, isabort, opts={}):
  
  Mutable default value should be avoided. It can be `backup=True` since we
  aren't processing command-level thingy here.
  
  And please make sure tests pass before sending out patches.
  
  > @@ -828,7 +829,8 @@
  > 
  >   else:
  >       ui.status(_('there will be no conflict, you can rebase\n'))
  >   finally:
  > 
  > - _origrebase(ui, repo, abort=True) +            opts = {'abort':True, 'no_backup':True} +            _origrebase(ui, repo, **opts)
  
  Maybe we can refactor the dryrun handling in a way we don't have to pass around
  the no_backup flag. IIUC, `_origrebase()` is a high-level function which has
  many parameter/state checks, but what we want is to cancel the in-memory
  session we've made just before. No user error should occur.
  
  > @@ -1588,7 +1590,10 @@
  > 
  >   1. Strip from the first rebased revision if rebased:
  > - repair.strip(repo.ui, repo, strippoints) +                if nobackup: +                    repair.strip(repo.ui, repo, strippoints, backup=False) +                else: +                    repair.strip(repo.ui, repo, strippoints)
  
  This can be written as `backup=not nobackup` or `backup=backup`.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel


More information about the Mercurial-devel mailing list