Alphare added inline comments. Alphare marked an inline comment as done. INLINE COMMENTS
> kevincox wrote in status.rs:23 > This doesn't appear to be consumed by the function so you should take a > `&[HgPathBuf]`. Or since I don't think you use the path either probably a > `&[HgPath]`. Done with `&[impl AsRef<HgPath> + Sync]` which is more generic. > kevincox wrote in status.rs:168 > This seems like a very complicated way to write this. Why not separate the > `match state` and do it first. Or put it in an `else` block? Furthermore why > do you have two separate `match state`? Why not handle them all in one > `match`? I could indeed probably simplify it. I'll update this part later today (Paris time) . > kevincox wrote in status.rs:240 > Do you need to do this to `root_dir`? The `walk_explicit` function seems to > only need `AsRef<Path>` so you should be able to just pass it. You might need > to update the argument to be a reference. I had to add a `Copy` trait bound for it to work, otherwise it would be moved by `walk_explicit` before usage in `stat_dmap_entries` > kevincox wrote in files.rs:112 > Isn't it better to avoid implementing this so that we get a compile error > rather than a runtime error? It is indeed a much better idea. I'll also send a patch for the other uses of `unimplemented!()` in the rest of the codebase REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7058/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7058 To: Alphare, #hg-reviewers, kevincox Cc: yuja, martinvonz, durin42, kevincox, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
