Re: [PATCH v2 00/11] Consolidate ref parsing code
On Wed, Oct 15, 2014 at 10:06 PM, Michael Haggerty wrote: > As far as I know, Duy isn't actively working on this, so I hope my > reroll is not unwelcome. I couldn't be happier that someone does the work for me and I still benefit from it ;) -- Duy -- 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
Re: [PATCH v2 00/11] Consolidate ref parsing code
On 10/16/2014 10:47 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> This is a rif on Duy's oldish patch series [1]. I started reviewing >> his patch series, but found that some of his patches did multiple >> things, making it harder to review. I started pulling it apart into >> smaller changes to aid my review, and I guess I got carried away :-/ > > Hmmm... > > You are aware of another large change in flight in the same area, > and after having reviewed that series a few times I was hoping that > you would have a better understanding of how ready the other topic > is. And then I see this series that conflicts with that other > topic. Is this your way to say that the other topic is not quite > ready? No, not at all. To be honest, I thought that the changes in this patch series were localized in an area different than Ronnie and Jonathan's patches were touching, but stupidly I didn't check for conflicts. Sorry about that. There is nothing urgent in this patch series, so I suggest we just put it back on ice and I will reroll after the dust has settled on rs/ref-transactions. The conflicts shouldn't be super hard to resolve (the series don't overlap much *conceptually*), but I'd rather not have to do it multiple times. Regarding rs/ref-transaction, I will reply on that thread. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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
Re: [PATCH v2 00/11] Consolidate ref parsing code
Junio C Hamano writes: > Michael Haggerty writes: > >> This is a rif on Duy's oldish patch series [1]. I started reviewing >> his patch series, but found that some of his patches did multiple >> things, making it harder to review. I started pulling it apart into >> smaller changes to aid my review, and I guess I got carried away :-/ > > Hmmm... > > You are aware of another large change in flight in the same area, > and after having reviewed that series a few times I was hoping that > you would have a better understanding of how ready the other topic > is. And then I see this series that conflicts with that other > topic. Is this your way to say that the other topic is not quite > ready? The last one was a rhetorical question and I regret that it came out a bit harsher than I intended to. I just wanted to see a bit better inter-developer coordination, that's all. Thanks. -- 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
Re: [PATCH v2 00/11] Consolidate ref parsing code
Michael Haggerty writes: > This is a rif on Duy's oldish patch series [1]. I started reviewing > his patch series, but found that some of his patches did multiple > things, making it harder to review. I started pulling it apart into > smaller changes to aid my review, and I guess I got carried away :-/ Hmmm... You are aware of another large change in flight in the same area, and after having reviewed that series a few times I was hoping that you would have a better understanding of how ready the other topic is. And then I see this series that conflicts with that other topic. Is this your way to say that the other topic is not quite ready? -- 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
[PATCH v2 00/11] Consolidate ref parsing code
This is a rif on Duy's oldish patch series [1]. I started reviewing his patch series, but found that some of his patches did multiple things, making it harder to review. I started pulling it apart into smaller changes to aid my review, and I guess I got carried away :-/ As far as I know, Duy isn't actively working on this, so I hope my reroll is not unwelcome. As far as I can tell, Duy's patch series was correct aside from a couple of minor cosmetic blemishes [2]. So if you want to accept Duy's original patch series, I hereby endorse it. This version does the following things beyond Duy's original: * Split commits up into smaller pieces. * Get rid of the MAXREFLEN limitation. * Rename the parse_ref() parameter from "ref" to "refname". * Inline resolve_gitlink_packed_ref() and handle_missing_loose_ref(). * Invert the "if" statement for dealing with symbolic references in parse_ref() to make the logic flow more linear. * Change a couple of "while" loops to "do..while". * Change resolve_refdup() to return strbuf_detach(&buf, NULL) instead of buf.buf directly (as suggested by Eric Sunshine). I retained Duy as author on commits that are derived straightforwardly from his. I hope I haven't broken any of them. I am calling this patch series v2 because I propose it as a successor to Duy's version. [1] http://thread.gmane.org/gmane.comp.version-control.git/254203/focus=254203 [2] Aside from the couple of things pointed out on the mailing list, * The parse_ref() parameter should be named "refname" instead of "ref", for consistency with other refs code. * The local variable "ref = result->buf" in resolve_ref() just obscures things and should be inlined. Michael Haggerty (5): resolve_ref_unsafe(): reverse the logic of the symref conditional handle_missing_loose_ref(): inline function resolve_gitlink_ref_recursive(): drop arbitrary refname length limit resove_gitlink_packed_ref(): inline function resolve_gitlink_ref(): remove redundant test Nguyễn Thái Ngọc Duy (6): strbuf_read_file(): preserve errno on failure handle_missing_loose_ref(): return an int resolve_ref_unsafe(): use skip_prefix() to skip over "ref:" refs.c: refactor resolve_ref_unsafe() to use strbuf internally refs.c: move ref parsing code out of resolve_ref() refs.c: rewrite resolve_gitlink_ref() to use parse_ref() cache.h | 12 +++ refs.c | 371 +++ strbuf.c | 7 +- 3 files changed, 200 insertions(+), 190 deletions(-) -- 2.1.1 -- 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
