gracinet marked 2 inline comments as done.
gracinet added a comment.

  > Does the class name actually matter? Personally I don't care if
  >  lazyancestors() function returns a LazyAncestors object. We'll anyway
  >  need a wrapper function to make pure ancestors and rustext ancestors
  >  compatible.
  
  Yes, that's in line with your other comments, whereas I was pursuing the
  goal of putting the whole `ancestor` module under responsibility of
  `policy.importmod` : in that case, it would have been necessary to have
  the same name.
  
  I think I still prefer to put all the dispatching in the `revlog` module
  (ie have it play the factory role) rather than put a factory function
  inside `ancestor`. Unless you disagree, I'm going to make a new revision
  going in that direction, ie
  
  - the class would be called `LazyAncestors`
  
  - the dispatching would stay in `revlog`
  
  - maybe for clarity, `ancestor.rustlazyancestors` should be renamed
  
  and in the next series (bindings for `MissingAncestors`), similar
  dispatching would occur in `incrementalmissingrevs`
  
  For now, `revlog` can choose according to whether `rustext` is (fully)
  importable (some other comment about that).
  
  Cheers,

REPOSITORY
  rHG Mercurial

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

To: gracinet, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to