Re: [PATCH 1/3] bundle-uri: drop unused 'uri' parameter
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Dec 20, 2022, 20:46 UTC
- Message-ID
- <221220.86k02lx0l7.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <75107325-bd28-1adf-b260-ba8846c63229@github.com>
On Tue, Dec 20 2022, Derrick Stolee wrote:
Show 21 quoted lines
> On 12/19/22 5:57 AM, Ævar Arnfjörð Bjarmason wrote: >> On Mon, Dec 12 2022, Derrick Stolee via GitGitGadget wrote: > >>> @@ -112,10 +112,10 @@ int fetch_bundle_uri(struct repository *r, const char *uri); >>> * bundle-uri protocol v2 verb) at the given uri, fetch and unbundle the >>> * bundles according to the bundle strategy of that list. >>> * >>> - * Returns non-zero if no bundle information is found at the given 'uri'. >>> + * It is expected that the given 'list' is initialized, including its >>> + * 'baseURI' value > >> The same goes for the added docs, that we "expect [that] 'list' is >> initialized" may be true, but that would have been true before we >> removed this unused parameter, so let's not stick that in this unrelated >> "UNUSED" change. > > It is _not_ unrelated. The 'uri' parameter looks like it should > be used to determine relative URLs for the included list. However, > this reasoning around the 'baseURI' value points out that we are > using that value _instead_ of the 'uri' value, which is why it > is safe to remove the parameter.
I'm saying that the function expected that baseURI to be initialized before, but with your rationale I think it's fine to also do it in one commit.
I also pointed out (and you elided) that we should still document the return code, do you agree that we should keep that?