D3849: rebase: refactor dryrun implementation

[yuja (Yuya Nishihara)]

yuja added a comment.


  > @yuja In this patch rebaseruntime is instantiated two times, one in
  >  _dryrunrebase and second in _origrebase, I don't know if it could make any
  >  problem although all tests are passing. Maybe _origrebase() also need some
  >  refactoring.
  
  Perhaps. I don't know if that really matters, but it's better to not keep
  two separate rbsrt objects alive.
  
  Can you reorder this as follows?
  
  1. extract _dryrunrebase() with no code change
  2. pass in rbsrt to _origrebase() (or new function extracted from _origrebase())
  3. replace _origrebase(abort=True) with rbsrt call
  
  > +    def _abortunfinishedrebase(self, backup=False, suppwarns=True):
  >  +        repo = self.repo
  >  +        with repo.wlock(), repo.lock():
  >  +            retcode = self._prepareabortorcontinue(isabort=True)
  >  +            return retcode
  
  If this is just calling _prepareabortorcontinue(), we wouldn't need a new
  function. And I think the lock should be held by the caller.

REPOSITORY
  rHG Mercurial

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

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