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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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?