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
