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?
