Re: [PATCH v5 02/14] Add new parameter "carry_data" for "show_object" function

From: Teng Long <dyroneteng@gmail.com>
Date: Sep 2, 2021, 11:08 UTC
Message-ID: <20210902110855.75687-1-dyroneteng@gmail.com>
In-Reply-To: <xmqqlf4oc4u1.fsf@gitster.g>

> Since this lacks <area>: prefix, "git shortlog" readers will have a
> hard time guessing which show_object() function this commit is
> about.

Sorry for the late reply. Agree, it will be fixed in next patch.

Show 16 quoted lines

> But it does not quite explain why we need another parameter to do
> so, which involves changing the function signature of many
> functions, instead of making show_data to point at a new structure
> type that holds the original data show_data used to carry plus
> another single void * member (or the set of members you'd be
> carrying into these functions using this new parameter).
> 
> I also find "carry_data" a meaningless name for the parameter.  All
> in-parameters into functions are used to carry some data into it
> after all.  The existing "show_data" at least makes a bit more
> sense; it contains data necessary for showing the object in these
> code paths.  If the purpose this new thing was introduced is to
> cache ownership relationship data, perhaps ownership_cache would be
> a more descriptive and understandable name (be it a new parameter to
> added to many functions, or a member to the new structure that
> replaces show_data).

Agree.

I think "show_data to point at a new structure" is a better idea. I will follow the idea and optimize the related code. By the way, the naming about "ownership_cache" will be introduced in the next patch too, unless I can find a better one t(-_-t).

Thanks.