martinvonz added inline comments. INLINE COMMENTS
> quark wrote in rebase.py:1004 > Line 1088 is not `isancestor`. I'm okay with having two functions here. Let > me know what do you prefer. It looks like it effectively is. You can rewrite it using isancestor(). The reason I brought this up at all was that cl.ancestor() returns a single ancestor even if there are multiple (as in criss-cross merges), which made me wonder what would happen if the wrong ancestor was returned. I don't think there can be a "wrong" ancestor for the uses here, but using isancestor() instead makes that clearer. Also, it seems like isancestor(a,b) can be made faster than ancestors(a,b)==a can because it can stop walking when when the rev is lower than a's rev. > quark wrote in rebase.py:1010 > Interesting. I didn't know that we treat p1 specially here. If so, users can > always construct an asymmetric case that feels wrong. For example, if we use > B or C as E's parent in the above graph, p1 may not get chosen in one of the > case. > > In this diff, it's Line 1091 trying to be conservative. I'll add tests and a > warning like https://phab.mercurial-scm.org/D340 (Diff 770) line 1115. I'm not sure I understand, but I don't think the user can influence the result (and I'm assuming you mean "as *D*'s parent", otherwise I can't make any sense of it). It should be fine to use either B' or C' as the new p1, as long as base is set to B or C (respectively). > quark wrote in rebase.py:1019 > Maybe `assert i == 0` is a better assertion here. If `nullrev` is considered > part of the DAG, then I think it makes sense for `adjustdest` to return > `dest` for p2. True, I can see the point about [nullrev,nullrev] being valid implying that any [X,X] should be valid. How about the rest of my comment? I didn't understand what you meant by the first part of you reply. > test-rebase-newancestor.t:287 > removing other > - note: merging f9daf77ffe76+ and 4c5f12f25ebe using bids from ancestors > a60552eb93fb and f59da8fc0fcf > - Sorry, I forgot to ask before, but why did this change? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D21 To: quark, durin42, #hg-reviewers Cc: martinvonz, durin42, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
