Re: [PATCH v4 3/3] rev-list: add commit object support in `--missing` option

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> +	if (revs->do_not_die_on_missing_objects)
>> +		oidset_init(&revs->missing_objects, 0);
>> +
>
> While we're initializing the new oidset, we never clear it. We should
> probably call `oidset_clear()` in `release_revisions()`. And if we
> unconditionally initialized the oidset here then we can also call
> `oiadset_clear()` unconditionally there, which should be preferable
> given that `oidset_init()` does not allocate memory when no initial size
> was given.

Yup, I used the conditional one to match the above, but initializing
unused oidset is cheap and frees us from having to worry about
mistakes.  I like your idea much better.

>> +
>> +	/* Missing objects to be tracked without failing traversal. */
>> +	struct oidset missing_objects;
>
> As far as I can see we only use this set to track missing commits, but
> none of the other objects. The name thus feels a bit misleading to me,
> as a reader might rightfully assume that it contains _all_ missing
> objects after the revwalk. Should we rename it to `missing_commits` to
> clarify?

Again, very good suggestion.

Thanks.