On 5/12/2019 11:44 PM, Junio C Hamano wrote: > "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> >> The write_commit_graph() and write_commit_graph_reachable() methods >> currently take two boolean parameters: 'append' and 'report_progress'. >> We will soon expand the possible options to send to these methods, so >> instead of complicating the parameter list, first simplify it. > > I think this change to introduce "flags" and pack these two into a > single parameter, even if there is no plan to add code that starts > using third and subsequent bits immediately. > > We are no longer adding anything beyond PROGRESS and APPEND in this > series, no? In this series, we are no longer expanding the options. I will add a flag when I update the incremental file format series. I can modify the message to no longer hint at an immediate addition. >> >> Collapse these parameters into a 'flags' parameter, and adjust the >> callers to provide flags as necessary. >> >> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> --- >> builtin/commit-graph.c | 8 +++++--- >> builtin/commit.c | 2 +- >> builtin/gc.c | 4 ++-- >> commit-graph.c | 9 +++++---- >> commit-graph.h | 8 +++++--- >> 5 files changed, 18 insertions(+), 13 deletions(-) >> >> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c >> index 2e86251f02..828b1a713f 100644 >> --- a/builtin/commit-graph.c >> +++ b/builtin/commit-graph.c >> @@ -142,6 +142,7 @@ static int graph_write(int argc, const char **argv) >> struct string_list *commit_hex = NULL; >> struct string_list lines; >> int result; >> + int flags = COMMIT_GRAPH_PROGRESS; > > Make it a habit to use "unsigned" not a signed type, when you pack a > collection of bits into a flag word, unless you are treating the MSB > specially, e.g. checking to see if it is negative is cheaper than > masking with MSB to see if it is set. Ah sorry. I missed this one after changing the parameter in your earlier feedback. >> ... >> result = write_commit_graph(opts.obj_dir, >> pack_indexes, >> commit_hex, >> - opts.append, >> - 1); >> + flags); >> ... >> -int write_commit_graph_reachable(const char *obj_dir, int append, >> - int report_progress) >> +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags) >> ... >> int write_commit_graph(const char *obj_dir, >> struct string_list *pack_indexes, >> struct string_list *commit_hex, >> - int append, int report_progress) >> + unsigned int flags) > > OK, so the receivers of the flags word know the collection is > unsigned; it's just the user of the API in graph_write() that gets > the signedness wrong. OK, easy enough to correct, I guess. >
