Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-05 Thread Michael Haggerty
On 01/04/2016 08:01 PM, Junio C Hamano wrote: > David Aguilar writes: > [...] >> My only comment is that it seems like having a single static global >> the_refs_backend seems like it should be avoided. >> [...] > > I think the ship is sailing in a different direction. The API

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-05 Thread Junio C Hamano
Michael Haggerty writes: > Actually, maybe we will never have to rewrite all callers. I rather > think that we will retain one simplified API for dealing with "*this* > repository's references" and a second for dealing with other repos' refs. Yeah, I think the similarity

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-04 Thread Ronnie Sahlberg
On Sat, Jan 2, 2016 at 4:06 PM, David Aguilar wrote: > Apologies for the late review, and this review should probably > go on patch 01 or 02 but I don't have it in my mbox atm... > > On Wed, Dec 02, 2015 at 07:35:08PM -0500, David Turner wrote: >> From: Ronnie Sahlberg

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-04 Thread Junio C Hamano
David Aguilar writes: >> diff --git a/refs.c b/refs.c >> index 9562325..b9b0244 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -1150,3 +1150,57 @@ int resolve_gitlink_ref(const char *path, const char >> *refname, >> { >> return the_refs_backend->resolve_gitlink_ref(path,

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-04 Thread Junio C Hamano
Ronnie Sahlberg writes: > Not commenting on whether this is the right direction or not. A global > variable holding a methods table might not be most aesthetic design, > but there are practicalities. > > However, that kind of change would change the function signatures for >

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-04 Thread Junio C Hamano
Jeff King writes: > As much as it would be nice to clean this up before moving to multiple > backends, though, I don't think we should make it a pre-requisite. This > is a difficult topic as it is, and I'd rather see us make incremental > improvement (backends, then hopefully more

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-04 Thread Jeff King
On Mon, Jan 04, 2016 at 12:26:01PM -0800, Junio C Hamano wrote: > An API enhancement to allow us to handle refs in multiple > repositories separately would be a very welcome move (it would get > rid of the hacky interface for_each_ref_in_submodule(), for one > thing), but even after that happens,

Re: [PATCH 03/16] refs: add methods for the ref iterators

2016-01-02 Thread David Aguilar
Apologies for the late review, and this review should probably go on patch 01 or 02 but I don't have it in my mbox atm... On Wed, Dec 02, 2015 at 07:35:08PM -0500, David Turner wrote: > From: Ronnie Sahlberg > > Signed-off-by: Ronnie Sahlberg >

[PATCH 03/16] refs: add methods for the ref iterators

2015-12-02 Thread David Turner
From: Ronnie Sahlberg Signed-off-by: Ronnie Sahlberg Signed-off-by: David Turner --- refs.c | 54 refs/files-backend.c | 41