Patchwork D3855: rebase: extract dryrun as a function

login
register
mail settings
Submitter phabricator
Date June 28, 2018, 8:10 p.m.
Message ID <differential-rev-PHID-DREV-xr6iptaw7vxwyfhnp2yj-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32490/
State Superseded
Headers show

Comments

phabricator - June 28, 2018, 8:10 p.m.
khanchi97 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  To avoid more number of indented blocks and make it easier to add
  additional functionality in dryrun, extracted as a function.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/rebase.py

CHANGE DETAILS




To: khanchi97, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - June 29, 2018, 1:02 p.m.
>          rbsrt = rebaseruntime(repo, ui, inmemory, opts)
>          with repo.wlock(), repo.lock():
>              try:
> -                overrides = {('rebase', 'singletransaction'): True}
> -                with ui.configoverride(overrides, 'rebase'):
> -                    _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt,
> -                                leaveunfinished=True, **opts)
> +                _dryrunrebase(ui, repo, rbsrt, **opts)
>              except error.InMemoryMergeConflictsError:
>                  ui.status(_('hit a merge conflict\n'))
>                  return 1

Oops, I meant the whole dryrun process could be extracted to a function
because it's getting bigger.

```
if dryrun:
    return _dryrunrebase(...)
else:
    ...
```

But feel free to ignore my suggestion if there's a reason to not move
everything.
phabricator - June 29, 2018, 1:03 p.m.
yuja added a comment.


  >   rbsrt = rebaseruntime(repo, ui, inmemory, opts)
  >   with repo.wlock(), repo.lock():
  >       try:
  > 
  > - overrides = {('rebase', 'singletransaction'): True}
  > - with ui.configoverride(overrides, 'rebase'):
  > - _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt,
  > - leaveunfinished=True, **opts) +                _dryrunrebase(ui, repo, rbsrt, **opts) except error.InMemoryMergeConflictsError: ui.status(_('hit a merge conflict\n')) return 1
  
  Oops, I meant the whole dryrun process could be extracted to a function
  because it's getting bigger.
  
    if dryrun:
        return _dryrunrebase(...)
    else:
        ...
  
  But feel free to ignore my suggestion if there's a reason to not move
  everything.

REPOSITORY
  rHG Mercurial

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

To: khanchi97, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - June 29, 2018, 6:02 p.m.
khanchi97 added a comment.


  In https://phab.mercurial-scm.org/D3855#60250, @yuja wrote:
  
  > >   rbsrt = rebaseruntime(repo, ui, inmemory, opts)
  > >   with repo.wlock(), repo.lock():
  > >       try:
  > > 
  > > - overrides = {('rebase', 'singletransaction'): True}
  > > - with ui.configoverride(overrides, 'rebase'):
  > > - _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt,
  > > - leaveunfinished=True, **opts) +                _dryrunrebase(ui, repo, rbsrt, **opts) except error.InMemoryMergeConflictsError: ui.status(_('hit a merge conflict\n')) return 1
  >
  > Oops, I meant the whole dryrun process could be extracted to a function
  >  because it's getting bigger.
  >
  >   if dryrun:
  >       return _dryrunrebase(...)
  >   else:
  >       ...
  >
  >
  > But feel free to ignore my suggestion if there's a reason to not move
  >  everything.
  
  
  Aha, I thought you were pointing out to only "try:" block for more no. of indented blocks. I will update this.

REPOSITORY
  rHG Mercurial

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

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

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -821,10 +821,7 @@ 
         rbsrt = rebaseruntime(repo, ui, inmemory, opts)
         with repo.wlock(), repo.lock():
             try:
-                overrides = {('rebase', 'singletransaction'): True}
-                with ui.configoverride(overrides, 'rebase'):
-                    _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt,
-                                leaveunfinished=True, **opts)
+                _dryrunrebase(ui, repo, rbsrt, **opts)
             except error.InMemoryMergeConflictsError:
                 ui.status(_('hit a merge conflict\n'))
                 return 1
@@ -848,6 +845,12 @@ 
     else:
         return _origrebase(ui, repo, **opts)
 
+def _dryrunrebase(ui, repo, rbsrt, **opts):
+    overrides = {('rebase', 'singletransaction'): True}
+    with ui.configoverride(overrides, 'rebase'):
+        return _origrebase(ui, repo, inmemory=True, rbsrt=rbsrt,
+                           leaveunfinished=True, **opts)
+
 def _origrebase(ui, repo, inmemory=False, leaveunfinished=False, rbsrt=None,
                 **opts):
     opts = pycompat.byteskwargs(opts)