Page MenuHomePhabricator

index: add a `rev` method (API)
ClosedPublic

Authored by marmoute on Nov 8 2019, 8:33 AM.

Details

Summary

The new index.rev(node) is to be preferred over using `node
index.nodemap[node]`.

This get us closer to be able to remove the nodemap attribute of the index.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

marmoute created this revision.Nov 8 2019, 8:33 AM
indygreg requested changes to this revision.Nov 8 2019, 3:06 PM

The code is fine. But since I've already left a number of comments on this series, I thought I'd ask about raising a better exception than RevlogError. I thought we had dedicated exceptions for revision not existing?

This revision now requires changes to proceed.Nov 8 2019, 3:06 PM
marmoute requested review of this revision.Nov 8 2019, 7:36 PM

The code is fine. But since I've already left a number of comments on this series, I thought I'd ask about raising a better exception than RevlogError. I thought we had dedicated exceptions for revision not existing?

This is the exception currently raised by nodemap access and recognised by higher level code (that turn it into a better exception). Maybe this should change ? but this is out of scope from this series in my opinion.

marmoute updated this revision to Diff 17822.Nov 8 2019, 8:10 PM
marmoute updated this revision to Diff 17837.Nov 9 2019, 12:15 AM

Perhaps it's best to bump the version number in this patch and the get_rev patch as well? I don't feel strongly, but I'll queue all the has_node patches to start with.

marmoute updated this revision to Diff 17883.Nov 9 2019, 8:13 AM
indygreg requested changes to this revision.Nov 9 2019, 10:45 AM

I'm fine ignoring the RevlogError quirk, since it already exists.

This revision now requires changes to proceed.Nov 9 2019, 10:45 AM
indygreg accepted this revision.Nov 9 2019, 10:46 AM
This revision is now accepted and ready to land.Nov 9 2019, 10:46 AM
This revision was automatically updated to reflect the committed changes.