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

Patrick Steinhardt [off-list ref] writes:

quoted

+	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.

quoted

+
+	/* 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.