Done thanks for the suggestions On Tue, May 3, 2022 at 4:07 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > Junio C Hamano <gitster@xxxxxxxxx> writes: > > Calvin Wan <calvinwan@xxxxxxxxxx> writes: > > > > > When the state machine in do_fetch_pack_v2() is in FETCH_CHECK_LOCAL, we > > > set a few v2-specific defaults. It will be helpful for a future patch to > > > have these defaults set if the initial state is not FETCH_CHECK_LOCAL. > > > > > > > This is a safe change since the initial state is currently always > > > FETCH_CHECK_LOCAL, so we're guaranteed to hit that code whether it's in > > > the state machine or not. > > > > What does "it" (which supposed to be able to be in the state machine > > and also not to be in the state matchine) in the last sentence refer > > to? > > I think the "it" refers to the initialization code that was moved. > > > The patch looks correct and I agree that this is a benign no-op > > because the initial value of state is FETCH_CHECK_LOCAL (i.e. the > > block of code moved here will execute pretty much as the first > > thing, and the relative order between that block and sorting of ref > > list should not matter). I just didn't understand the explanation > > given by the patch why it is safe. > > > > Thanks. > > Agreed - I think the commit message would be better off if it was worded > something like: > > There are some initialization steps that need to be done at the start > of the execution of the do_fetch_pack_v2() state machine. Currently, > these are done in the FETCH_CHECK_LOCAL state, which is the initial > state. > > However, a subsequent patch will allow for another initial state, > while still requiring these initialization steps. Therefore, move > these initialization steps to before the state machine, so that they > are run regardless of the initial state. > > I think this description suffices for a reviewer to see that it is safe, > but if you want, you can add: > > Note that there is no change in behavior, because we're moving code > from the beginning of the first state to just before the execution of > the state machine.
