D6260: rust-discovery: takefullsample() core implementation

gracinet (Georges Racinet) phabricator at mercurial-scm.org
Mon May 6 11:38:37 EDT 2019


gracinet marked an inline comment as done.
gracinet added a comment.


  I think it's maybe ready to land in that form. In the future, I'd like to put this `ParentsIterator` in a more generic place, and IMHO, it should become part of an `AbstractGraph` trait, that could live in a `graph` module.
  
  Ideally, we should even be able to generically reverse these graphs, so that the children iteration that's been done in this discovery process would just be the same, as seen from `update_sample`, but it's maybe too much asking.

INLINE COMMENTS

> kevincox wrote in discovery.rs:137
> Can't you just call `self.next()` in both cases?

Yes that's what I actually did first, and then I tried this variant to see if it's faster. It may be a bit, but nothing worth doing in a first inclusion.

> kevincox wrote in discovery.rs:280
> Generally I would just do this.
> 
>   for &rev in self.undecided.as_ref().unwrap() {

yes, same as in parent commit. Indeed it's a nicer way to iterate on references to `Copy` objects

> kevincox wrote in discovery.rs:281
> Same.

for this one, I can actually use `ParentsIterator`, now, spares me the `NULL_REVISION` check

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list