On 7/15/2022 6:27 AM, Phillip Wood wrote: >> We can test that this works by rewriting the todo-list several times in >> the course of a rebase. Check that each ref is locked or unlocked for >> updates after each todo-list update. We an also verify that the ref >> update fails if a concurrent process updates one of the refs after the >> rebase process records the "locked" ref location. > > Thanks for adding this and taking the time to write some good tests. When adding a new update-ref command to the todo list it would be nice to check if the ref is already checked. We could print a warning or force the user to re-edit the todo list as we do if they delete a pick and rebase.missingcommitscheck == error I agree that that would be a helpful feature instead of the user discovering the issue at the end of the rebase (as they do with the message from patch 12). > The checks below are quadratic but we don't expect that many refs so that should be fine. I don't think it is worth the complexity of building a hash table or whatever at this stage. A hash table would be the natural approach if we see users having trouble from such high use of the feature. I think both of these concerns would be excellent for a follow-up, since they would shave off some rough edges. I hesitate to add them to this series since it has been growing quite a bit already. Thanks, -Stolee
