D5945: rust: itering less on MissingAncestors.bases for max()

kevincox (Kevin Cox) phabricator at mercurial-scm.org
Tue Feb 12 17:20:41 EST 2019


kevincox added inline comments.

INLINE COMMENTS

> gracinet wrote in ancestors.rs:41
> I must confess to have hesitated a bit on that one. On one hand, it would work, but semantically if ever a new "special" revision -2 is introduced, this could become problematic. On the other hand, there are probably a few areas that already depend on the fact that NULL_REVISION is the smallest one.
> 
> From the graph side of things, NULL_REVISION being a common ancestor of all revisions, so it must be smaller or equal than all, so it doesn't matter much.
> 
> Option<Revision> was merely a simple way to be sure not to be wrong, I'm open to suggestions.

If you don't want to depend on the value of NULL_REVISION you can use the minimum value for the type backing revision.

Other special revisions shouldn't be relevant because you shouldn't be comparing them as the max anyways.

That being said the currents state is fine. So if you think it makes more sense that sounds good to me.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5945

To: gracinet, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list