Re: [PATCH v3 3/3] reftable: prevent 'update_index' changes after adding records

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

Patrick Steinhardt <ps@pks.im> writes:

>> +static void t_reftable_invalid_limit_updates(void)
>> +{
>> +	struct reftable_ref_record ref = {
>> +		.refname = (char *) "HEAD",
>> +		.update_index = 1,
>> +		.value_type = REFTABLE_REF_SYMREF,
>> +		.value.symref = (char *) "master",
>> +	};
>> +	struct reftable_write_options opts = {
>> +		.default_permissions = 0660,
>> +	};
>
> Nit: it's unnecessary to pass write options. Other than that the test
> looks good to me, and this nit isn't worth a reroll.

This write_options opts is used later in this call.

	err = reftable_new_stack(&st, dir, &opts);

and "git grep reftable_new_stack" finds many hits, almost all in
t/unit-tests/ hierarchy, only two among many of them passing NULL as
the "use the default set of options" signal.  And majority of them
initialize their opts like so:

	struct reftable_write_options opts = { 0 };

So I agree that this one should pass NULL to be more explicit that
we do not exercise any special features from the API, but so should
many other existing callers that pass such meaningless &opts, I
would think.

Thanks.