martinvonz added a comment.
In https://phab.mercurial-scm.org/D4788#72444, @pulkit wrote: > I am not sure about this one. I was unable to think of a reason why we need to do this dirstate dance in non-ellipses cases. @martinvonz @durin42 do you know why we do this? What I usually do when I can't understand why something is needed: remove the code and run tests :) That will often tell you there there was in fact a reason for the code and give you some hints what that reason is. Of course, in some cases it won't tell you that and then you'll have to figure out if it's just untested or actually useless. Do you mean lines 279-280 of `hgext/narrow/narrowcommands.py` before this patch? If you remove those lines, you will see that `test-narrow{,patterns,widen,widen-no-ellipsis}.t` fail. You were right about the reason for the first `setparent()` call, btw. The reason for the code is so we move back to the old commit once we've restored the temporarily stripped commits. INLINE COMMENTS > narrowcommands.py:254-260 > + # for now we assume that if a server has ellipses enabled, we will be > + # exchanging ellipses nodes. In future we should add ellipses as a client > + # side requirement (maybe) to distinguish a client is shallow or not and > + # then send that information to server whether we want ellipses or not. > + # Theoretically a non-ellipses repo should be able to use narrow > + # functionality from an ellipses enabled server > + ellipsesremote = wireprotoserver.ELLIPSESCAP in remote.capabilities() You have add the requirement earlier in the series, so you can just use that here? (I suspect this is a leftover from an earlier version of this patch.) > narrowcommands.py:279 > > - with ui.uninterruptable(): > - ds = repo.dirstate > - p1, p2 = ds.p1(), ds.p2() > - with ds.parentchange(): > - ds.setparents(node.nullid, node.nullid) > + with ui.uninterruptable(), repo.ui.configoverride(overrides, 'widen'): > common = commoninc[0] I think I'd prefer to not silence developer warnings more than necessary even if that means duplicating the `repo.ui.configoverride(overrides, 'widen')` (i.e. I'd prefer to keep that together with the `wrappedextraprepare` block). REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D4788 To: pulkit, durin42, #hg-reviewers Cc: martinvonz, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
