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

[Derrick Stolee <derrickstolee@github.com>]

On 12/8/2022 1:36 AM, Jeff King wrote:
> On Wed, Dec 07, 2022 at 10:27:06AM -0500, Derrick Stolee wrote:
> 
>>> 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.
> 
> That makes sense. I've queued a patch locally to remove it (since
> locally I build with -Wunused-parameters), which will eventually make
> its way to the list.
> 
>>> 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.
> 
> Sounds good (and again, I've queued something locally, but if you beat
> me to it, it's easy to drop mine).
> 
> Note that your series hit 'next' (which is how I noticed it), so there
> usually would not be a "next version". Though we will rewind
> post-release, so there may still be an opportunity (I didn't follow the
> topic closely enough to know if you might want to re-roll for other
> reasons).

I noticed that after reading this round of review, so I'll be preparing
some fixes on top. I noticed some UNUSED that would be necessary from
earlier parts of the bundle URI work, and you've probably already queued
those changes.

Since I no longer plan to re-roll this series, I'd be happy to review
your queued annotations, and I'll focus on the other fixups.

Thanks,
-Stolee