D6632: rust-dirstate: rust implementation of dirstatemap

[kevincox (Kevin Cox)]

kevincox added inline comments.
kevincox accepted this revision.

INLINE COMMENTS

> dirstate_map.rs:191
> +                        changed = true;
> +                        *entry = DirstateEntry {
> +                            mtime: MTIME_UNSET,

Can you just do `entry.mtime = MTIME_UNSET`?

> dirstate_map.rs:288
> +    pub fn set_parents(&mut self, parents: DirstateParents) {
> +        self.parents = Some(parents.clone());
> +        self.dirty_parents = true;

This `.clone()` looks unnecessary. If it is necessary you should probably accept a `&DirstateParents`.

> dirstate_map.rs:306
> +
> +        if !self.dirty_parents {
> +            self.set_parents(parents.to_owned());

It isn't clear to me why we don't `set_parents` if `dirty_parents`. This would make me think that `parents` is lazily calculated however `set_parents` sets `parents` and `dirty_parents` which would imply that this is not the case.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6632/new/

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

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