On Mon, Dec 12 2022, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>
> The 'uri' parameter of fetch_bundle_list() was added early in
> development, but is not required since the 'list' parameter includes a
> 'baseURI' member. Remove the 'uri' parameter and make this expectation
> explicit.
Makes sense, and this is a straightforward commit, but...
> diff --git a/bundle-uri.h b/bundle-uri.h
> index b2c9c160a52..29b0c98ee65 100644
> --- a/bundle-uri.h
> +++ b/bundle-uri.h
> @@ -68,8 +68,8 @@ struct bundle_list {
> * In the case of the 'bundle-uri' protocol v2 command, the base
> * URI is the URI of the Git remote.
> *
> - * Otherewise, the bundle list was downloaded over HTTP from some
> - * known URI.
> + * Otherwise, the bundle list was downloaded over HTTP from some
> + * known URI. 'baseURI' is set to that value.
...here we have a stray typo fix, not called out in the commit
message. I think that can pass, but if you're re-rolling I think any
such "while-at-it" would be better split into their own commits.
But more importantly:
> *
> * The baseURI is used as the base for any relative URIs
> * advertised by the bundle list at that location.
> @@ -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
At first sight this seems like a regression. Why have we stopped
documenting the exit code?
But looking again is this because in 7cc47a980ac (bundle-uri: download
bundles from an advertised list, 2022-12-05) you accidentally
copy/pasted the documentation for fetch_bundle_uri(), and this was never
applicable to this function?
Even then, not documenting the code we return now is a regression. If it
was wrong before, let's make it correct, not stop discussing it
entirely.
In either case this is another while-at-it entirely unrelated to the
$subject here of dropping an unused parameter.
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.
> */
> int fetch_bundle_list(struct repository *r,
> - const char *uri,
> struct bundle_list *list);
>
> /**
