Re: [PATCH v3 10/11] bundle-uri: download bundles from an advertised list

From: Derrick Stolee <derrickstolee@github.com>
Date: Dec 7, 2022, 15:27 UTC
Message-ID: <affbc458-d4f5-525f-d431-5ec1d489afc8@github.com>
In-Reply-To: <Y5CNo5kyByIHDVYh@coredump.intra.peff.net>

On 12/7/2022 7:57 AM, Jeff King wrote:

Show 26 quoted lines

> On Mon, Dec 05, 2022 at 05:50:38PM +0000, Derrick Stolee via GitGitGadget wrote:
> 
>> +int fetch_bundle_list(struct repository *r, const char *uri, struct bundle_list *list)
>> +{
>> +	int result;
>> +	struct bundle_list global_list;
>> +
>> +	init_bundle_list(&global_list);
>> +
>> +	/* If a bundle is added to this global list, then it is required. */
>> +	global_list.mode = BUNDLE_MODE_ALL;
>> +
>> +	if ((result = download_bundle_list(r, list, &global_list, 0)))
>> +		goto cleanup;
>> +
>> +	result = unbundle_all_bundles(r, &global_list);
>> +
>> +cleanup:
>> +	for_all_bundles_in_list(&global_list, unlink_bundle, NULL);
>> +	clear_bundle_list(&global_list);
>> +	return result;
>> +}
> 
> The "uri" parameter in this function is unused. I'm not sure if that's
> indicative of a bug or missing feature (e.g., could it be the base for a
> relative url?), or if it's just a leftover from development.

Thanks for your careful eye. This 'uri' is indeed not needed. I think it
was initially there for relative URIs, but the given 'list' is expected
to have that value initialized. I'll make it clear in the doc comment.

Show 5 quoted lines

> If the latter, I'm happy to add it to my list of cleanups.
> 
> There are a couple other unused parameters in this series, too, but they
> are all in virtual functions and must be kept. I'll add them to my list
> of annotations.

Your UNUSED annotations exist in my tree, so I'll try my best to update them in the next version.

Thanks, -Stolee