Re: [PATCH 8/8] builtin/repack.c: pass `--refs-snapshot` when writing bitmaps

On Fri, Sep 10 2021, Taylor Blau wrote:

+struct midx_snapshot_ref_data {
+	struct tempfile *f;

Style/readability: Spare more than one byte for a variable name in a
struct, maybe file, or tmpfile?

+	fprintf(data->f->fp, "%s%s\n", data->preferred ? "+" : "",
+		oid_to_hex(oid));

Just an idea: Maybe the file could be lines of "+\tOID\n" instead of
"+OID\n"? Lends itself more naturally to extension, use with the likes
of string_list_split() etc.

+		for_each_string_list_item(item, preferred) {
+			for_each_ref_in(item->string, midx_snapshot_ref_one, &data);
+		}

Cheap style commenst I know, but throughout this series there's an odd
mixture of sometimes adding braces for one-lines, sometimes not, or in
the case of that "else ;" I commented on not for something *much* bigger
:)

+		if (midx_snapshot_refs(refs_snapshot) < 0)
+			die(_("could not take a snapshot of references"));

Seems like this die() should be moved into the midx_snapshot_refs()
function and that function should use die_errno(), not die(), also it
uses close_tempfile_gently(), shouldn't save the errno on failure there,
clean up the tempfile, and then die?