Derrick Stolee <[email protected]> writes:
> +int commit_contains(struct ref_filter *filter, struct commit *commit,
> + struct commit_list *list, struct contains_cache *cache)
This is a symbol that is used to be file-local private. Is it named
appropriately in the new context, which is "globally visible
throughout the system"? The convention to call into it now must be
documented a lot better (e.g. how should list/cache etc are to be
prepared?).
> +{
> + if (filter->with_commit_tag_algo)
> + return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
> + return is_descendant_of(commit, list);
> +}
> diff --git a/commit-reach.h b/commit-reach.h
> index 35ec9f0ddb..986fb388d5 100644
> --- a/commit-reach.h
> +++ b/commit-reach.h
> @@ -2,42 +2,24 @@
> #define __COMMIT_REACH_H__
>
> #include "commit.h"
> +#include "commit-slab.h"
> +#include "ref-filter.h"
>
> -struct commit_list *get_merge_bases_many(struct commit *one,
> - int n,
> - struct commit **twos);
> -struct commit_list *get_merge_bases_many_dirty(struct commit *one,
> - int n,
> - struct commit **twos);
> -struct commit_list *get_merge_bases(struct commit *one, struct commit *two);
> -struct commit_list *get_octopus_merge_bases(struct commit_list *in);
> -
> -/* To be used only when object flags after this call no longer matter */
> -struct commit_list *get_merge_bases_many_dirty(struct commit *one, int n,
> struct commit **twos);
> -
> -int is_descendant_of(struct commit *commit, struct commit_list *with_commit);
> -int in_merge_bases_many(struct commit *commit, int nr_reference, struct
> commit **reference);
> -int in_merge_bases(struct commit *commit, struct commit *reference);
> -
> +int ref_newer(const struct object_id *new_oid, const struct object_id
> *old_oid);
>
> /*
> - * Takes a list of commits and returns a new list where those
> - * have been removed that can be reached from other commits in
> - * the list. It is useful for, e.g., reducing the commits
> - * randomly thrown at the git-merge command and removing
> - * redundant commits that the user shouldn't have given to it.
> - *
> - * This function destroys the STALE bit of the commit objects'
> - * flags.
The above removal of lines is sloppy; they are mostly duplicates in
commit.h that should never have been moved here in the first place,
no?
> + * Unknown has to be "0" here, because that's the default value for
> + * contains_cache slab entries that have not yet been assigned.
> */
> -struct commit_list *reduce_heads(struct commit_list *heads);
> +enum contains_result {
> + CONTAINS_UNKNOWN = 0,
> + CONTAINS_NO,
> + CONTAINS_YES
> +};
Are these names specific enough, or were they OK in the limited
context inside ref-filter but now are overly broad as globally
visible names? I suspect it might be the latter.
