This is an archive of the discontinued Mercurial Phabricator instance.

revlog: replace descendant(b, a) by isdescendantrev(a, b) (API)
ClosedPublic

Authored by martinvonz on Jul 11 2018, 8:09 PM.

Details

Summary

The "is" is to match "isancestor" and to make it clear that it doesn't
return a descendant. The "rev" is to make it clear that it's not about
nodeids (unlike e.g. isancestor()). The argument order change is just
seems more natural (and makes isancestor() less confusing).

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.

Event Timeline

martinvonz created this revision.Jul 11 2018, 8:09 PM

This doesn't feels simpler, would it be possible to simply rename descendant into isancestorrev without changing the order of the arguments?

Also it would be useful for extensions to have a deprecation warning at least for 1 cycle?

This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Jul 12 2018, 9:22 AM
This doesn't feels simpler, would it be possible to simply rename `descendant` into `isancestorrev` without changing the order of the arguments?

While I've queued this without reading any comments (I hate junk mails
from Phabricator), I second the removal/deprecation of isdescendant*()
in favor of isancestor*().

In D3929#61325, @yuja wrote:
This doesn't feels simpler, would it be possible to simply rename `descendant` into `isancestorrev` without changing the order of the arguments?

While I've queued this without reading any comments (I hate junk mails
from Phabricator), I second the removal/deprecation of isdescendant*()
in favor of isancestor*().

That's fine with me. Kind of ironic since someone very recently went in the opposite direction and wrote isancestor() in terms of descendant() :) I'll send a patch.