rebase: add the --inmemory option flag; assign a wctx object for the rebase
ClosedPublic

Authored by phillco on Oct 26 2017, 12:48 AM.

Details

Summary

In the future, the --inmemory flag might be deprecated in favor of something more
intelligent (for example, always rebasing in-memory if the working copy parent
isn't in the rebaseset). But we might keep it as a way to explicitly force IMM
on or off.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
phillco updated this revision to Diff 4042.Dec 1 2017, 3:09 AM
dlax requested changes to this revision.Dec 1 2017, 4:01 AM
dlax added a subscriber: dlax.

In the future, the --inmemory flag might be deprecated in favor of something more intelligent

Perhaps the option should be flagged experimental then?

Also, would be nice to have some test coverage.

hgext/rebase.py
738

Not sure this is needed.

This revision now requires changes to proceed.Dec 1 2017, 4:01 AM
durin42 added a subscriber: durin42.Dec 7 2017, 2:47 PM

I've gone through this series and landed some of the easy stuff. Let's get the comments on this one addressed and rebase the rest and see if we can get it in before everyone goes on Christmas vacations?

phillco updated this revision to Diff 4178.Dec 7 2017, 4:22 PM
phillco updated this revision to Diff 4196.Dec 7 2017, 4:29 PM

Perhaps the option should be flagged experimental then?

Good point, thanks.

durin42 accepted this revision.Dec 7 2017, 4:31 PM
This revision was automatically updated to reflect the committed changes.
dlax added inline comments.Dec 8 2017, 3:43 AM
hgext/rebase.py
738

This is useless because "inmemory" will be in opts and because you wrote opts.get('inmemory', False) in rebaseruntime.__init__() above.

dlax added a comment.Dec 8 2017, 5:01 AM

dlax (Denis Laxalde) wrote:

Also, would be nice to have some test coverage.

Unless I missed it, there's still no test at all for the --inmemory
option in this series.

smf added a subscriber: smf.Dec 11 2017, 6:59 PM

dlax (Denis Laxalde) <phabricator@mercurial-scm.org> writes:

dlax requested changes to this revision.
dlax added a comment.
This revision now requires changes to proceed.

> In the future, the --inmemory flag might be deprecated in favor of something more intelligent

Perhaps the option should be flagged experimental then?

Also, would be nice to have some test coverage.

We can't easily get rid of command-line options so this should be
'experimental.inmemory' or similar. Also, I view inmemory as an
implementation detail that shouldn't be exposed to the user.

Phil is planning to follow up with the config knob, and I hope to test this aggressively at Google and make it default if we don't find any bugs.

smf added a comment.Dec 11 2017, 7:24 PM

durin42 (Augie Fackler) <phabricator@mercurial-scm.org> writes:

durin42 added a comment.

Phil is planning to follow up with the config knob, and I hope to test this aggressively at Google and make it default if we don't find any bugs.

Same for me here (it's what we already do basically).

Given that most users will be using the config knob (and eventually, nothing at all), I can just send a followup to rename to experimental.inmmemory. Nobody is using it yet.

hgext/rebase.py
738

Thanks. I'll send a followup.