Page MenuHomePhabricator

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

Authored by marmoute on Fri, Nov 8, 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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Fri, Nov 8, 8:33 AM
indygreg requested changes to this revision.Fri, Nov 8, 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.Fri, Nov 8, 3:06 PM
marmoute requested review of this revision.Fri, Nov 8, 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.Fri, Nov 8, 8:10 PM
marmoute updated this revision to Diff 17837.Sat, Nov 9, 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.Sat, Nov 9, 8:13 AM
indygreg requested changes to this revision.Sat, Nov 9, 10:45 AM

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

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