Re: [GSoC][PATCH v6 00/11] ref consistency check infra setup

> Hello,
> 
> shejialuo <shejialuo@gmail.com> writes:
> 
> > Hi All:
> >
> > This version follows the Junio's advice. Instead of creating the
> > following data structure:
> >
> > 	struct fsck_options {
> > 		enum fsck_type {
> > 			FSCK_OBJECTS,
> > 			FSCK_REFS,
> > 			...
> > 		} t;
> > 		union {
> > 			struct fsck_objects_options objects;
> > 			struct fsck_refs_options refs;
> > 		} u;
> > 	};
> >
> > I simply use the combination idea where "fsck_options" will incorporate
> > "fsck_objects_options" and "fsck_refs_options". Karthik has told me that
> > I should balance the job I should does and the extensibility for future.
> > So I use the most clear way to do this. Also Junio has said:
> >
> 
> If I understood Junio's comments correctly, he was drawing out the point
> about if we even need the separation of options for refs. Since the only
> option we're adding is a verbose:
> 
>     struct fsck_refs_options {
>     	unsigned verbose:1;
>     };
> 
> wouldn't it be better if we simply amended `fsck_options` as so:
> 
>     diff --git a/fsck.h b/fsck.h
>     index 6085a384f6..ea97f48acc 100644
>     --- a/fsck.h
>     +++ b/fsck.h
>     @@ -135,6 +135,7 @@ struct fsck_options {
>      	fsck_walk_func walk;
>      	fsck_error error_func;
>      	unsigned strict:1;
>     +	unsigned verbose_refs:1;
>      	enum fsck_msg_type *msg_type;
>      	struct oidset skiplist;
>      	struct oidset gitmodules_found;
> 
> Your approach seems to take a different path though, where we create a
> new route of creating two new structs, one for refs and another for
> objects and adding both to fsck_objects. If we're doing this, wouldn't
> it be better to use the enum+union idea, like Junio mentioned? That way
> we would have clarity around which type it represents.
>