Hello,
shejialuo <shejialuo@xxxxxxxxx> 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.
[snip]
Attachment:
signature.asc
Description: PGP signature
