- User Since
- Apr 18 2019, 7:59 AM (74 w, 2 d)
Thu, Sep 17
Maybe adding some performance numbers in the commit message would be useful and motivating, seeing as it is very fast.
Good stuff, thanks for removing the cruft I introduced. :D
Minor nits, this looks good.
Both remarks are nits and can be addressed later if need be, this looks good otherwise.
Good enough for now, thanks.
I'll probably send a follow-up for the typo and maybe see about removing the .into() calls.
Fri, Sep 11
Thu, Sep 10
Thanks for taking the time to do this.
Other than my "nit but please do it" comment, this looks fine.
Sun, Sep 6
Mon, Aug 31
Fri, Aug 28
Overall this looks good. I have obviously left many comments, but a lot of them are nits, and it's a huge patch.
This is my partial review, as indicated by my last comment, I'll be resuming it later today.
Tue, Aug 25
Fri, Aug 21
What do you mean by "mishandling removed files"? The fold map is there to give a mapping between the normalized (internal) representation of file paths and the actual ones on disk. As much as I hate the dirstate code, this is a useful part of the software that is triggered depending on your filesystem and paths used.
Aug 18 2020
@indygreg Does my justification seem reasonable? Could we get both patches accepted?
Aug 12 2020
Aug 3 2020
Maybe add an example with | head for completeness?
To reviewers: this was my changeset, I'm not sure how authorship is attributed but I... accept the revision of course.
Typo in the title: s/simply/simplify/
@acezar Do you remember the kind of performance difference we had when adding this? I think it would be interesting to add it to the commit message.
To add to this change, it's also entirely possible that we may never need the trait altogether.
Jul 30 2020
@martinvonz this is inside the dirstatemap variant defined in case we use the Rust version. I agree that a small commit description is needed, otherwise this seems fine.
Jul 29 2020
Jul 23 2020
Note to reviewers: this series will be updated soon with a lot of changes from IRL discussions we had.
Jul 15 2020
I'm not a big fan of the small-but-non-obvious variable names in this series, but they predate your patches, so maybe for another time.
Thanks for this series
Thanks again for refactoring
Thanks for refactoring first!
Not a fan of the wording, as you said... Maybe "became empty as <rev>:<hash>"?
I seems like a good idea, but pinging @marmoute on this since he might have more perspective on that subject.
Jul 1 2020
Jun 29 2020
Jun 25 2020
Jun 24 2020
Jun 22 2020
Maybe send another patch that updates contrib/heptapod-ci.yml?
nit: "it's a wild used" -> "it's a wildly used" and "it can deal with bytes"... technically it deals with OS encoding. Which is good for our purposes, but is slightly different.
Looks good. If I could nitpick some more, the description talks about WTF8, when it's UTF8 (because we're talking about String and not OsString).
(I know this is a misfire, but just so that this isn't taken by a committed)
The title is invalid