On Wed, Jul 29, 2015 at 3:38 PM, Matthieu Moy
<Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> On Tue, Jul 28, 2015 at 7:18 PM, Matthieu Moy
>> <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>>> Christian Couder <christian.couder@xxxxxxxxx> writes:
>>>
>>>> On Tue, Jul 28, 2015 at 8:56 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>>>>
>>>>> +static void ref_array_append(struct ref_array *array, const char *refname)
>>>>> +{
>>>>> + size_t len = strlen(refname);
>>>>> + struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);
>>>>> + memcpy(ref->refname, refname, len);
>>>>> + ref->refname[len] = '\0';
>>>
>>> This looks very much like new_ref_array_item, except that the later also
>>> takes an objectname parameter. I find it suspicious that you leave the
>>> objectname field uninitialized.
>>>
>>
>> Well the objectname is not required here, the idea is to retain the way branch.c
>> works.
>>
>>> Why is this code not calling new_ref_array_item?
>>>
>>
>> Because no objectname is there.
>
> You do have the object_id in the scope of the call-site, so why not use
> it?
>
> (Well, in any case, do as you think is best, it's temporary throw-away
> code, we shouldn't loose too much time polishing it)
>
Also the fact the new_ref_array_item() is static and not shared. Wouldn't make
sense to make it an API only for a single commit, when the code itself
is temporary.
>>> The function disapears in the next commit, but I also think that this
>>> function deserves to exist in ref-filter.{c,h} and remain after the end
>>> of the series.
>>>
>>
>> Why though? I don't see the need when new_ref_array_item() is present.
>
> OK, if the function is specificly for "append an item but leave the
> objectname uninitialized", it's too specific to be useful somewhere
> else. But then, make it more explicit in the name of the function and/or
> in a comment.
>
Will add a comment :)
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html