Re: [PATCH v3 1/2] [GSOC] ref-filter: add objectsize to used_atom

From: ZheNing Hu <adlternative@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: ZheNing Hu via GitGitGadget <gitgitgadget@gmail.com>,
	Git List <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Hariom Verma <hariom18599@gmail.com>,
	Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH v3 1/2] [GSOC] ref-filter: add objectsize to used_atom
Date: Thu, 13 May 2021 17:04:36 +0800	[thread overview]
Message-ID: <CAOLTT8T9iwAk1vyQVa_kM5zdym6-UJ0Fi6gWJp=YnSAVOiCDsw@mail.gmail.com> (raw)
In-Reply-To: <xmqqpmxveee9.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> 于2021年5月13日周四 上午7:11写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > Since "objectsize:size" is composed of two parts,
> > "type:attribute". However, the original implementation
>
> Y is missing in the above "Since X, Y".  Because it is composed of
> two parts, what?
>
> Dropping "Since " would make the sentence work better.  Also, I
> think it should be "objectsize:disk".
>

Oh, here is my typo.

> > did not decouple the two parts "type" and "attribute" well,
> > we still need to judge separately whether the atom is
> > "objectsize" or "objectsize:disk" in `grab_common_values()`.
>
> Perhaps
>
>         When the support for "objectsize:disk" was bolted onto the
>         existing support for "objectsize", it didn't follow the
>         usual pattern for handling "atomtype:modifier", which reads
>         the <modifier> part just once while parsing the format
>         string, and store the parsed result in the union in the
>         used_atom structure, so that the string form of it does not
>         have to be parsed over and over at runtime (e.g. in
>         grab_common_values()).
>

That’s great.

Thanks.
--
ZheNing Hu