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

khanchi97 (Sushil khanchi) phabricator at mercurial-scm.org
Mon Jun 25 12:06:02 EDT 2018


khanchi97 added a comment.


  In https://phab.mercurial-scm.org/D3827#59878, @yuja wrote:
  
  > >   > > - retcode = rbsrt._prepareabortorcontinue(abortf) +            # If in-memory, means aborting during dry-run, no need to backup +            backup = not rbsrt.inmemory +            retcode = rbsrt._prepareabortorcontinue(abortf, backup=backup)
  > >   >
  > >   > This seems confusing and is probably wrong since we wouldn't overwrite
  > >   >  `inmemory` to `False` if in-memory rebase were resumable.
  > >   >
  > >   > I think explicit `backup` flag is less bad.
  > >   
  > >   
  > >   @yuja Can I pass a indicator type flag to _origrebase() just to indicate that we are in dryrun and then I can use that flag to set values for `backup` and `suppwarns` (suppress warning when aborting rebase). I think it would be better than explicitly passing flags for each case. What do you say?
  >
  > Sounds good.
  >
  > A cleaner (but not simple) approach would be to stop calling `_origrebase()`
  >  twice, and instead refactor `_origrebase()` and/or `rebaseruntime` to support
  >  dry-run operation.
  
  
  Sounds interesting. Let me try this.
  Thanks for review :)

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