D3849: rebase: refactor dryrun implementation

Yuya Nishihara yuya at tcha.org
Wed Jun 27 09:27:04 EDT 2018


> @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.


More information about the Mercurial-devel mailing list