Teng Long <dyroneteng@xxxxxxxxx> writes:
> Subject: Re: [PATCH v5 02/14] Add new parameter "carry_data" for "show_object" function
Since this lacks <area>: prefix, "git shortlog" readers will have a
hard time guessing which show_object() function this commit is
about.
> During the pack-objects process, "show_object" function will be called
> to find the object and show the process("show_object_fn" in
> "list-object.h"), the function definition contains three parameters:
>
> 1. struct object *obj(contains object type, flags, and oid).
> 2. const char *name(the object name).
> 3. void *show_data(function to show progress info).
>
> This commit adds a new parameter: "void *carry_data", the reason is
> mainly based on scalability and performance considerations when showing
> an object, space for time, avoid costly temporary calculations in the
> "show" phase. For example, carry the ownership relationship between
> blob or tree object and the referred commit to avoid redundant and
> expensive calculations.
The above explains what we want to carry around extra data for
(i.e. compute something in one place, and use it later somewhere
else)
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).
Thanks.
