> Currently, complete_action(), used by builtin/rebase.c to start a new > rebase, calls sequencer_continue() to do it. Before the former calls > pick_commits(), it > > - calls read_and_refresh_cache() -- this is unnecessary here as we've > just called require_clean_work_tree() in complete_action() require_clean_work_tree() and read_and_refresh_cache() seem to be different functions - can you explain why running the former is a good substitute for running the latter? > - calls read_populate_opts() -- this is unnecessary as we're starting a > new rebase, so `opts' is fully populated My comment from [1] has not been addressed. Quoting it here: > So complete_action() (the function modified in this commit) is called > only by do_interactive_rebase() (in builtin/rebase.c), which is only > called by run_rebase_interactive() (in builtin/rebase.c) when command is > ACTION_NONE, so indeed, we're starting a new rebase. But where the > options fully populated? I see that in do_interactive_rebase(), it is > initialized with get_replay_opts(), but that seems different from > read_populate_opts(). [1] https://lore.kernel.org/git/20191119204146.168001-1-jonathantanmy@xxxxxxxxxx/ > - loads the todo list -- this is unnecessary as we've just populated > the todo list in complete_action() Both functions indeed have their own todo lists that they pass to pick_commits(), but I don't see (at least, by glancing at the code) that both these todo lists are identical. > - commits any staged changes -- this is unnecessary as we're starting a > new rebase, so there are no staged changes > - calls record_in_rewritten() -- this is unnecessary as we're starting > a new rebase. OK - I don't know enough about the rebase mechanism to verify these, but these seem reasonable to me.
