On Wed, 2015-12-23 at 06:33 +0100, Michael Haggerty wrote:
> On 12/03/2015 01:35 AM, David Turner wrote:
> > Alternate refs backends might not need the refs/heads directory and
> > so
> > on, so we make ref db initialization part of the backend. We also
> > might need to initialize ref backends themselves, so we'll add a
> > method for that as well.
> >
> > Signed-off-by: David Turner <dtur...@twopensource.com>
> > ---
> > builtin/init-db.c | 14 ++++----------
> > refs.c | 8 +++++++-
> > refs.h | 4 +++-
> > refs/files-backend.c | 23 +++++++++++++++++++++++
> > refs/refs-internal.h | 4 ++++
> > 5 files changed, 41 insertions(+), 12 deletions(-)
> >
> > [...]
> > diff --git a/refs.c b/refs.c
> > index 9a2fed7..bdeb276 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -22,13 +22,14 @@ struct ref_be *refs_backends = &refs_be_files;
> > /*
> > * This function is used to switch to an alternate backend.
> > */
> > -int set_refs_backend(const char *name)
> > +int set_refs_backend(const char *name, void *data)
>
> The purpose of the data argument is rather mysterious at this point,
> especially since set_refs_backend() still doesn't have any callers. I
> hope that will become clearer later in the series. Nevertheless, it
> would be nice for its use to be described in the docstring (which
> should
> preferably be moved to the header file).
Will fix.
> > struct ref_be {
> > struct ref_be *next;
> > const char *name;
> > + ref_backend_init_fn *init_backend;
> > + ref_backend_init_db_fn *init_db;
> > ref_transaction_commit_fn *transaction_commit;
> > ref_transaction_commit_fn *initial_transaction_commit;
> >
> >
>
> Your naming seems inconsistent here. I would have expected a
> "files_init_backend()" function to satisfy the typedef
> "ref_backend_init_backend_fn" or "ref_init_backend_fn", not
> "ref_backend_init_fn". Or, conversely, for the function implementing
> "ref_backend_init_fn" to be called "files_init()".
>
> More generally, it would be nice to have a consistent naming pattern
> between (1) the name of the typedef, (2) the name of the member in
> "struct ref_be", (3) the names of concrete, backend-specific
> implementations of the functions.
I'll change this to "refs_init_backend_fn *init_backend" and
"refs_init_db_fn *init_db" (since we already have an init_db function),
and make any other similar changes I happen to notice.
On the naming in general, I am somewhat at the mercy of history here.
For example, git presently has functions named ref_transaction_commit
(object_verb) and create_reflog (verb_object). So nothing I do will be
consistent with everything. I could, of course, do some initial
commits to clean up the naming, but that would create code churn.
--
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
