D21: rebase: rewrite core algorithm (issue5578) (issue5630)

quark (Jun Wu) phabricator at mercurial-scm.org
Mon Aug 14 11:06:34 EDT 2017


quark added inline comments.

INLINE COMMENTS

> martinvonz wrote in rebase.py:1070
> In this case, maybe we shouldn't have added `p` to `bases` above? Maybe that block should be moved after this loop? Maybe it would be easier to keep track of the bases in a list that's initialized to [None,None]? Or maybe if the order of `newps` always matches `oldps`, can `bases` be calculated later and not mixed in with the calculation of `newps`?

No. New parents having an ancestor relationship does not mean the old parents have that relationship. It seems your test cases in `test-rebase-obsolete.t` would capture the behavior difference.

> martinvonz wrote in rebase.py:1134
> Thanks for catching this case and warning about it! I wonder if we should even error out early instead? Even if we decide to do that, feel free to leave it for a followup.

An error makes sense to me. But it changes more tests. I'll do that after https://phab.mercurial-scm.org/D340.

It's hard to detect it early without in-memory-changelog to do a dry-run rebase.

> martinvonz wrote in rebase.py:1136
> It would probably be good to include the nodeid here just like we do on the line above, especially since the revision number may point to something else after the rebase is complete (if not using obsmarkers).

This was just me being lazy. Ideally all these `%d:%s` thing should be a template config so we can hide revision numbers in production.

REPOSITORY
  rHG Mercurial

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

To: quark, durin42, #hg-reviewers
Cc: martinvonz, durin42, mercurial-devel


More information about the Mercurial-devel mailing list