( )⚙ D6230 rust-dagops: roots

This is an archive of the discontinued Mercurial Phabricator instance.

rust-dagops: roots
ClosedPublic

Authored by gracinet on Apr 12 2019, 2:33 PM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHGbe0733552984: rust-dagops: roots
Summary

Unsuprisingly, the algorithm is much easier than for heads, provided
we work on a set in the first place.

To improve the signature, a trait for set-likes object would be useful,
but that's not an immediate concern.

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

gracinet created this revision.Apr 12 2019, 2:33 PM
kevincox accepted this revision.Apr 14 2019, 3:51 PM
kevincox added inline comments.
rust/hg-core/src/dagops.rs
96

I think it is more clear to write .iter().filter(|p| *p != NULL_REVISION).all(|p| !revs.contains(p)). This separates the NULLs from the revs you care about and makes that condition easier to parse.

gracinet marked an inline comment as done.Apr 15 2019, 6:10 AM
gracinet updated this revision to Diff 14741.
gracinet added inline comments.Apr 15 2019, 6:11 AM
rust/hg-core/src/dagops.rs
96

Thanks for the suggestion.

Anticipating on a discussion to have later, I'd like eventually the Graph::parents() returns an iterator. The advantages will be made obvious by the (not submitted yet) final changesets of the` PartialDiscovery` implementation in hg-core. In other words, NULL_REVISION and the fact to have exactly two parents should be considered implementation details of the underlying storage.

kevincox added inline comments.Apr 15 2019, 6:23 AM
rust/hg-core/src/dagops.rs
96

That sounds great. NULL_REVISION always seemed quite messy to me. It would definitely be best to filter it out as soon as possible.

This revision was automatically updated to reflect the committed changes.