D3849: rebase: refactor dryrun implementation
yuja (Yuya Nishihara)
phabricator at mercurial-scm.org
Thu Jun 28 08:32:24 EDT 2018
yuja added a comment.
Queued, thanks.
I found a couple of nits. Can you send a follow up?
> if dryrun:
>
> + leaveunfinished = True
> + inmemory = True
> + rbsrt = rebaseruntime(repo, ui, inmemory, opts)
>
> try:
> overrides = {('rebase', 'singletransaction'): True}
> with ui.configoverride(overrides, 'rebase'):
>
> - _origrebase(ui, repo, inmemory=True, leaveunfinished=True,
> - **opts) + _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt, + leaveunfinished=leaveunfinished, **opts) except error.InMemoryMergeConflictsError: ui.status(_('hit a merge conflict\n')) return 1 else: ui.status(_('there will be no conflict, you can rebase\n')) return 0 finally:
> - _origrebase(ui, repo, abort=True) + with repo.wlock(), repo.lock(): + rbsrt._prepareabortorcontinue(isabort=True)
We need a lock covering the whole dryrun process. Otherwise, another hg could
interrupt the dryrun before aborting.
Since we'll need one more indented block, I think it's time to extract dryrun
to a function.
> -def _origrebase(ui, repo, inmemory=False, leaveunfinished=False, **opts):
> +def _origrebase(ui, repo, inmemory=False, leaveunfinished=False, rbsrt=None,
> + **opts):
>
> opts = pycompat.byteskwargs(opts)
>
> - rbsrt = rebaseruntime(repo, ui, inmemory, opts) + if not rbsrt: + rbsrt = rebaseruntime(repo, ui, inmemory, opts)
I think it's slightly better to split function than adding optional `rbsrt`
argument.
> with repo.wlock(), repo.lock():
> # Validate input and define rebasing points
Perhaps we can extract the post-lock block to new function, and the dryrun
function will call it in place of `_origrebase()`:
rbsrt = rebaseruntime(repo, ui, inmemory, opts)
with repo.wlock(), repo.lock():
try:
do in-memory rebase
finally:
rbsrt._prepareabortorcontinue(isabort=True)
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3849
To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel
More information about the Mercurial-devel
mailing list