All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Turner <dturner@twopensource.com>
To: Stefan Beller <sbeller@google.com>
Cc: git mailing list <git@vger.kernel.org>,
	ronnie sahlberg <ronniesahlberg@gmail.com>
Subject: Re: RFC/Pull Request: Refs db backend
Date: Tue, 23 Jun 2015 16:04:55 -0400	[thread overview]
Message-ID: <1435089895.28466.65.camel@twopensource.com> (raw)
In-Reply-To: <CAGZ79kap++fZx3X0D95d35XioRURU468xATDZpWHDOAPapAh+Q@mail.gmail.com>

On Tue, 2015-06-23 at 10:16 -0700, Stefan Beller wrote:
> > The db backend code was added in the penultimate commit; the rest is
> > just code rearrangement and minor changes to make alternate backends
> > possible.  There ended up being a fair amount of this 
> > rearrangement,  but the end result is that almost the entire git 
> > test suite runs under the db backend without error (see below for 
> details).
> 
> Looking at the end result in refs-be-db.c it feels like there are more
> functions in the refs_be_db struct, did this originate from other 
> design choices? IIRC Ronnie wanted to have as least functions in 
> there as possible, and share as much of the code between the 
> databases, such that the glue between the db and the refs code is 
> minimal.

I didn't actually spend that much time reading Ronnie's backend code.
My code aims to be extremely thoroughly compatible.  I spent a ton of
time making sure that the git test suite passed.  I don't know if an
alternate approach would have been as compatible.

The requirement for reflog storage did complicate things a bit.

I also didn't see a strong need to abstract the database, since LMDB is
common, widely compatible, and tiny.  

> Some random comments from looking over the branch briefly:
> 
> In the latest commit, (refs: tests for db backend), I am unsure about 
> the copyright annotations. At least a sole "Copyright (c) 2007 Junio C
> Hamano" doesn't make sense to me. ;)

Will fix, thanks.

> Typo in commit message "bisect: use refs insfrastructure for 
> BISECT_START"

Will fix, thanks.

> Some commits contain a ChangeId, which is a Gerrit leftover. :(

Those were leftover from Ronnie's patches; since you are a Googler and
you think we don't need them, I'll remove them. 

  reply	other threads:[~2015-06-23 20:05 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-23  0:50 RFC/Pull Request: Refs db backend David Turner
2015-06-23  5:36 ` Junio C Hamano
2015-06-23 10:23   ` Duy Nguyen
2015-06-23 18:47     ` David Turner
2015-06-23 17:29   ` David Turner
2015-06-23 11:47 ` Jeff King
2015-06-23 13:10   ` Duy Nguyen
2015-06-24  8:51     ` Jeff King
2015-06-23 18:18   ` David Turner
2015-06-24  9:14     ` Jeff King
2015-06-24 17:29       ` David Turner
2015-06-24  6:09   ` Shawn Pearce
2015-06-24  9:49     ` Jeff King
2015-06-25  1:08       ` brian m. carlson
2015-06-24 10:18     ` Duy Nguyen
2015-06-23 15:51 ` Michael Haggerty
2015-06-23 19:53   ` David Turner
2015-06-23 21:27     ` Michael Haggerty
2015-06-24 17:31       ` David Turner
2015-06-23 21:35     ` David Turner
2015-06-23 21:41       ` Junio C Hamano
2015-06-23 17:16 ` Stefan Beller
2015-06-23 20:04   ` David Turner [this message]
2015-06-23 20:10     ` Randall S. Becker
2015-06-23 20:22       ` David Turner
2015-06-23 20:27         ` Randall S. Becker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1435089895.28466.65.camel@twopensource.com \
    --to=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.