On 7/15/2022 6:27 AM, Phillip Wood wrote:

quoted

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