Re: [PATCH v3 13/14] commit-graph: rename 'split_commit_graph_opts'

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Tue, Aug 11, 2020 at 04:52:11PM -0400, Taylor Blau wrote:
> In the subsequent commit, additional options will be added to the
> commit-graph API which have nothing to do with splitting.
> 
> Rename the 'split_commit_graph_opts' structure to the more-generic
> 'commit_graph_opts' to encompass both.

Good.  Note, however, that write_commit_graph() has a 'flags'
parameter as well, and when a feature is enabled via this 'flags',
then first a corresponding 'ctx->foo' field is set, and that
'ctx->foo' is checked while computing and writing the commit-graph.
With the generic options struct some other feature will be enabled via
the 'opts->bar' field, so simply 'ctx->opts->bar' is checked while
writing the commit-graph.

With the generic options struct there really is no need for a separate
flags parameter, the values in the flags can be stored in the options
struct, and we can eliminate this inconsistency instead of adding even
more.


> diff --git a/commit-graph.h b/commit-graph.h
> index ddbca1b59d..af08c4505d 100644
> --- a/commit-graph.h
> +++ b/commit-graph.h
> @@ -109,7 +109,7 @@ enum commit_graph_split_flags {
>  	COMMIT_GRAPH_SPLIT_REPLACE          = 2
>  };
>  
> -struct split_commit_graph_opts {
> +struct commit_graph_opts {
>  	int size_multiple;
>  	int max_commits;
>  	timestamp_t expire_time;
        enum commit_graph_split_flags flags;

While this was 'struct split_commit_graph_opts *split_opts' it was
clear what kind of flags were in this 'flags' field.  Now that the
struct is generic it's not clear anymore, so perhaps it should be
renamed as well (e.g. 'split_flags'), or even turned into a couple of
bit fields.

> @@ -124,12 +124,12 @@ struct split_commit_graph_opts {
>   */
>  int write_commit_graph_reachable(struct object_directory *odb,
>  				 enum commit_graph_write_flags flags,
> -				 const struct split_commit_graph_opts *split_opts);
> +				 const struct commit_graph_opts *opts);
>  int write_commit_graph(struct object_directory *odb,
>  		       struct string_list *pack_indexes,
>  		       struct oidset *commits,
>  		       enum commit_graph_write_flags flags,
> -		       const struct split_commit_graph_opts *split_opts);
> +		       const struct commit_graph_opts *opts);
>  
>  #define COMMIT_GRAPH_VERIFY_SHALLOW	(1 << 0)
>  
> -- 
> 2.28.0.rc1.13.ge78abce653
>