On Fri, 2015-12-11 at 15:39 -0800, Junio C Hamano wrote:
> David Turner <dtur...@twopensource.com> writes:
>
> > struct ref_be {
> > struct ref_be *next;
> > const char *name;
> > ref_transaction_commit_fn *transaction_commit;
> > +
> > + pack_refs_fn *pack_refs;
> > + peel_ref_fn *peel_ref;
> > + create_symref_fn *create_symref;
> > +
> > + resolve_ref_unsafe_fn *resolve_ref_unsafe;
> > + verify_refname_available_fn *verify_refname_available;
> > + resolve_gitlink_ref_fn *resolve_gitlink_ref;
> > };
>
> This may have been pointed out in the previous reviews by somebody
> else, but I think it is more customary to declare a struct member
> that is a pointer to a customization function without leading '*',
> i.e.
>
> typedef TYPE (*customize_fn)(ARGS);
>
> struct vtable {
> ...
> cutomize_fn fn;
> ...
> };
>
> in our codebase (cf. string_list::cmp, prio_queue::compare).
The previous review was here:
http://permalink.gmane.org/gmane.comp.version-control.git/279062
Michael wrote:
> Hmmm, I thought our convention was to define typedefs for functions
> themselves, not for the pointer-to-function; e.g.,
>
> typedef struct ref_transaction *ref_transaction_begin_fn(struct
> strbuf *err);
>
> (which would require `struct ref_be` to be changed to
>
> ref_transaction_begin_fn *transaction_begin;
And you agreed. So I changed it. Do you want me to change it back?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
