On 07/11/17 03:38, Junio C Hamano wrote: > Phillip Wood <phillip.wood@xxxxxxxxxxxx> writes: > >> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> >> >> Move print_commit_summary() from builtin/commit.c to sequencer.c so it >> can be shared with other commands. The function is modified by >> changing the last argument to a flag so callers can specify whether >> they want to show the author date in addition to specifying if this is >> an initial commit. > > A movement of a long function like this one really is easier if you > did not make any other unnecessary change in the same patch and then > made the change as a follow-up. I'm not sure what you mean by unnecessary, the original code called a file-local function author_date_is_interesting(). That had to be changed in order to move the code, I guess it would have been clearer to make that change first and then move the modified code to sequencer.c in a separate commit. > The end result seemed sane. > > Do not use signed int as a collection of bits "flags", as it makes > readers wonder if you are going to do some clever thing by treating > the topmost bit somewhat special (e.g. "if (flags < 0)"). Unless > you are indeed doing something clever like that, use "unsigned int" > instead. > Thanks, I'll change it to an unsigned int. Best Wishes Phillip
