Re: [GSoC][PATCH v6 00/11] ref consistency check infra setup
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Jul 2, 2024, 10:33 UTC
- Message-ID
- <CAOLa=ZSFTcMn+CsEcE1G5hnW37rw81zcUaXBywVY7xbBN7mhkA@mail.gmail.com>
- In-Reply-To
- <ZoLHtmOKTfxMSxvw@ArchLinux>
Hello,
shejialuo <shejialuo@gmail.com> writes:
Show 22 quoted lines
> 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]