On 8/18/2020 8:09 PM, Jonathan Tan wrote:
>> Instead of writing a new commit-graph in every 'git maintenance run
>> --auto' process (when maintenance.commit-graph.enalbed is configured to
>
> s/enalbed/enabled/
>
>> +/* Remember to update object flag allocation in object.h */
>> +#define PARENT1 (1u<<16)
>
> Why this name? "SEEN" seems perfectly fine.
For some reason I thought that there might be issues using
SEEN (1u<<0) but trying it locally does not seem to be a
problem. I think I've just been bit by how revision.c uses
it a bit too often. The use here is independent enough to
not cause problems.
>> +static int num_commits_not_in_graph = 0;
>> +static int limit_commits_not_in_graph = 100;
>> +
>> +static int dfs_on_ref(const char *refname,
>> + const struct object_id *oid, int flags,
>> + void *cb_data)
>> +{
>
> [snip]
>
>> +static int should_write_commit_graph(void)
>> +{
>> + int result;
>> +
>> + git_config_get_int("maintenance.commit-graph.auto",
>> + &limit_commits_not_in_graph);
>> +
>> + if (!limit_commits_not_in_graph)
>> + return 0;
>> + if (limit_commits_not_in_graph < 0)
>> + return 1;
>> +
>> + result = for_each_ref(dfs_on_ref, NULL);
>
> I don't like introducing the mutable global num_commits_not_in_graph
> especially when there seems to be at least one easy alternative (e.g.
> putting it in cb_data) but I know that this is a pattern than existing
> code.
I should have done this right in the first place. Thanks for
catching it.
-Stolee
