Byron
Add `it blame-copy-royal`
This is a draft PR. It is mostly intended for early feedback, in case there’s any. I plan on addressing the remaining TODOs over the course of the coming days. I also plan on providing more context. :-)
Thanks for making this happen!
Early feedback could be that this seems to be a breaking change in gix-blame, which should then go into a separate commit. I didn't look at it beyond that though.
@Byron I think I’ve now reached a point where it makes sense for you to start reviewing!
I tried to add as much context as possible in comments. I hope that’s enough context, let me know if you need more! I also tried to have each commit only touch a single crate. Feel free to squash if you want!
What I wasn’t sure about was the command’s name. This initial version basically just copies the name from copy-royal because it seems somehow related. Another option that might work is extract-blame-history.
I will continue testing the PR in the background. My gut feeling tells me that, at this point, any issues I might still find could also be issues in the blame implementation itself.
The generated script I thought I could get a little bit faster, with no results worth bragging about.
The initial run of
cargo run -p internal-tools --release -- blame-copy-royal . out README.mdtook 38s, and by the end it was … 37s :D. I tried to remove the tagging and replace with checking out commits… until I realised my mistake :D.
You’re right, the script hasn’t really been optimized for performance. :-) But the intermediate representation Vec<BlameScriptOperation> is supposed to make certain optimizations easier. For example, you can remove all instances of CheckoutTag(x) that immediately follow CreateTag(x). You can also remove all CreateTag(x) that are only ever referenced in a single CheckoutTag(x) that immediately follows the corresponding CreateTag(x). I didn’t do this right away as I wanted to wait for the results of your review first. I also think this is something that could be done in a follow-up commit.
I also think this is something that could be done in a follow-up commit.
Only if it's fun for you :)! Performance doesn't matter too much there, because it did we'd serialise the data as RON and restore using gix entirely. It would be done in a second or less, without trying even.
Only if it's fun for you :)! Performance doesn't matter too much there, because it did we'd serialise the data as RON and restore using
gixentirely. It would be done in a second or less, without trying even.
I was already starting to wonder why we’re using a shell script at all. 😄 Is it because we want git to be involved to make sure the created repository can serve as a baseline?
No, the shell script is needed to easily integrate them into the current test-system.
However, if ever needed, gix-testtools could of course learn how to restore such states more efficiently.
Meanwhile, if you feel your testing is slowed down by the script, it's no problem to implement a Rust version in addition to what's currently there.