"Stefan Beller via GitGitGadget" <gitgitgad...@gmail.com> writes:

> From: Stefan Beller <sbel...@google.com>
>
> Signed-off-by: Stefan Beller <sbel...@google.com>
> Signed-off-by: Derrick Stolee <dsto...@microsoft.com>
> ---
>  builtin/replace.c | 8 ++++----
>  refs.c            | 9 ++++-----
>  refs.h            | 2 +-
>  replace-object.c  | 5 +++--
>  4 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/replace.c b/builtin/replace.c
> index ef22d724b..d0b1cdb06 100644
> --- a/builtin/replace.c
> +++ b/builtin/replace.c
> @@ -39,7 +39,8 @@ struct show_data {
>       enum replace_format format;
>  };
>  
> -static int show_reference(const char *refname, const struct object_id *oid,
> +static int show_reference(struct repository *r, const char *refname,
> +                       const struct object_id *oid,
>                         int flag, void *cb_data)
>  {
>       struct show_data *data = cb_data;
> @@ -56,9 +57,8 @@ static int show_reference(const char *refname, const struct 
> object_id *oid,
>                       if (get_oid(refname, &object))
>                               return error("Failed to resolve '%s' as a valid 
> ref.", refname);
>  
> -                     obj_type = oid_object_info(the_repository, &object,
> -                                                NULL);
> -                     repl_type = oid_object_info(the_repository, oid, NULL);
> +                     obj_type = oid_object_info(r, &object, NULL);
> +                     repl_type = oid_object_info(r, oid, NULL);
>  
>                       printf("%s (%s) -> %s (%s)\n", refname, 
> type_name(obj_type),
>                              oid_to_hex(oid), type_name(repl_type));
> diff --git a/refs.c b/refs.c
> index 2513f77ac..5700cd468 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1478,12 +1478,11 @@ int refs_for_each_fullref_in(struct ref_store *refs, 
> const char *prefix,
>       return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
>  }
>  
> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void *cb_data)
> +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void 
> *cb_data)
>  {
> -     return do_for_each_ref(get_main_ref_store(r),
> -                            git_replace_ref_base, fn,
> -                            strlen(git_replace_ref_base),
> -                            DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
> +     return do_for_each_repo_ref(r, git_replace_ref_base, fn,
> +                                 strlen(git_replace_ref_base),
> +                                 DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
>  }
>  
>  int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
> diff --git a/refs.h b/refs.h
> index 80eec8bbc..a0a18223a 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -317,7 +317,7 @@ int for_each_fullref_in(const char *prefix, each_ref_fn 
> fn, void *cb_data,
>  int for_each_tag_ref(each_ref_fn fn, void *cb_data);
>  int for_each_branch_ref(each_ref_fn fn, void *cb_data);
>  int for_each_remote_ref(each_ref_fn fn, void *cb_data);
> -int for_each_replace_ref(struct repository *r, each_ref_fn fn, void 
> *cb_data);
> +int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void 
> *cb_data);

With a signature change like this, any change that introduces new
call to for_each_replace_ref() using eac_ref_fn() would get
compilation error, so this is minimally correct.

Two things that bothersome are that

 - for_each_tag/branch/remote_ref() and for_each_replace_ref() now
   work and look quite differently.

 - existing users of for_each_replace_ref() who were all happy
   working in the_repository have to pass it explicitly, even
   thought they do not have any need to.

In this case, even if you introduced for_each_replace_ref_in_repo(),
making for_each_replace_ref() a thin wrapper that always uses
the_repository is a bit more cumbersome than just a simple macro.

But it *is* doable (you'd need to use a wrapping structure around
cb_data), and a developer who case about maintainability during API
transition would have taken pains to do so.  A bit dissapointing.

>  int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data);
>  int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
>                        const char *prefix, void *cb_data);
> diff --git a/replace-object.c b/replace-object.c
> index 801b5c167..017f02f8e 100644
> --- a/replace-object.c
> +++ b/replace-object.c
> @@ -6,7 +6,8 @@
>  #include "repository.h"
>  #include "commit.h"
>  
> -static int register_replace_ref(const char *refname,
> +static int register_replace_ref(struct repository *r,
> +                             const char *refname,
>                               const struct object_id *oid,
>                               int flag, void *cb_data)
>  {
> @@ -25,7 +26,7 @@ static int register_replace_ref(const char *refname,
>       oidcpy(&repl_obj->replacement, oid);
>  
>       /* Register new object */
> -     if (oidmap_put(the_repository->objects->replace_map, repl_obj))
> +     if (oidmap_put(r->objects->replace_map, repl_obj))
>               die("duplicate replace ref: %s", refname);
>  
>       return 0;

Reply via email to