kevincox added inline comments. kevincox accepted this revision. INLINE COMMENTS
> status.rs:41 > + last_normal_time: i64, > +) -> (HgPathBuf, Dispatch) { > + let filename = filename.as_ref(); Every exit path is just `filename.to_owned()`. It would be better to just let the caller do that if required and return only the `Dispatch`. > status.rs:64 > + // of detecting changes. (issue2608) > + let range_mask = 0x7fffffff; > + I would create a helper function. fn mod_compare(a: i32, b: impl Into<i32>) -> bool { let b = b.into(); a != b && a != b & range_mask } Also I think the condition is easier to read as: a & i32::max_value() != b & i32::max_value() > status.rs:71 > + if size >= 0 > + && (size_changed || mode_changed) > + || size == -2 // other parent I would break this out into another local. let metadata_changed = size >= 0 && (size_changed || mode_changed); let other_parent = size == -2; if metadata_change || other_parent || copy_map.contains_key(filename) As a bonus this also makes the &&/|| precedence obvious. > status.rs:72 > + && (size_changed || mode_changed) > + || size == -2 // other parent > + || copy_map.contains_key(filename) Can we put this magic number in a constant anywhere? > status.rs:105 > + state: EntryState, > +) -> (HgPathBuf, Dispatch) { > + let filename = filename.as_ref(); Same, we don't need to return the path. > status.rs:127 > + last_normal_time: i64, > +) -> std::io::Result<Vec<(HgPathBuf, Dispatch)>> { > dmap.par_iter() It would be best to return the iterator here, this way we don't need to collect into a `Vec` just to throw it away. You can do the final sorting using a parallel `for_each`. However even if we end up doing a collection into Vec we can do that at the place of use rather than here which forces it upon all callers. > status.rs:133 > + |(filename, entry)| -> Option< > + std::io::Result<(HgPathBuf, Dispatch)> > > { Unless I am missing something this function always returns `Some(_)`. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7254/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7254 To: Alphare, #hg-reviewers, kevincox Cc: durin42, kevincox, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel