Re: [PATCH v2 0/2] remote-bzr: couple of fixes
On Sun, May 5, 2013 at 3:58 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >>> So do you want to queue these on top of the "massive" in 'next', not >>> directly on 'master'? >> >> If they apply on master, master. But I'm confused, are the massive >> changes not going to graduate to master? Because if not, I should >> cherry-pick the safest changes, as there's a lot of good stuff there. > > I think we discussed and agreed that we would ship it in 1.8.3 if we > hear positive feedback from Emacs folks, and my understanding is > that I was waiting for you to give me a go-ahead once that happens. Yeah, and I just said everything seems to be fine. There's only one more patch that would be good to have that I still haven't cleaned up. > It is entirely up to you to add these two on top of that "massive" > stuff, their fate decided by feedback from Emacs folks, or apply > these as "much safer than those we need to hear from them; we can > verify their validity and safety ourselves without knowing the real > world projects that use the program" patches. > > The impression I was getting from your response "I hear it breaks > for some of them without the patch but I haven't seen the breakage > myself" is that it is safer to group 2/2 as part of the rest of the > series, but as I heard in the same message that you heard Emacs > folks are happy with the entire series, so it wouldn't make much of > a difference either way. > > Will apply these two to the tip of the "massive" stuff, and merge > the result before the next -rc. Cool, I think that's the best approach. I'll send the last patch later today. Cheers. -- Felipe Contreras -- 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 0/2] remote-bzr: couple of fixes
Felipe Contreras writes: >> So do you want to queue these on top of the "massive" in 'next', not >> directly on 'master'? > > If they apply on master, master. But I'm confused, are the massive > changes not going to graduate to master? Because if not, I should > cherry-pick the safest changes, as there's a lot of good stuff there. I think we discussed and agreed that we would ship it in 1.8.3 if we hear positive feedback from Emacs folks, and my understanding is that I was waiting for you to give me a go-ahead once that happens. It is entirely up to you to add these two on top of that "massive" stuff, their fate decided by feedback from Emacs folks, or apply these as "much safer than those we need to hear from them; we can verify their validity and safety ourselves without knowing the real world projects that use the program" patches. The impression I was getting from your response "I hear it breaks for some of them without the patch but I haven't seen the breakage myself" is that it is safer to group 2/2 as part of the rest of the series, but as I heard in the same message that you heard Emacs folks are happy with the entire series, so it wouldn't make much of a difference either way. Will apply these two to the tip of the "massive" stuff, and merge the result before the next -rc. 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 0/2] remote-bzr: couple of fixes
On Sun, May 5, 2013 at 2:03 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> On Sun, May 5, 2013 at 1:33 PM, Junio C Hamano wrote: >>> Felipe Contreras writes: >>> The previous version had an indentation bug (did I mention I hate python?). A few fixes to be applied on top of the massive changes already queued. Nothing major. >>> >>> [2/2] may not matter much in the context of my tree (people would >>> use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3 >>> from my tree ;-), >> >> Maybe, but if even if they have the latest git, pushing a tag will >> fail miserably, and with the patch it would fail nicely :) >> >>> but [1/2] sounds like it is a good thing to have >>> in 1.8.3 (not "on top of that 'massive' series"). >>> >>> Assuming the "otherwise some version of bzr might barf" problem is >>> that repo.generate_revision_history() in those versions may not >>> apply str() to its first parameter and the caller is expected to >>> pass a string there, or something? >> >> No, there's no change to repo.generate_revision_history(), because we >> already convert the elements of the array to strings, it's the other >> callers of Marks::to_rev() that see a change, namely code that pushes >> to a remote, I think. >> >> And BTW, they are already strings, but unicode strings, because they >> come from a json file, somehow bazaar doesn't like that, but it works >> fine in my machine without the patch. Shrugs. >> >> Also, the emacs developers seem to be fine with all these changes, >> there's only one patch pending that I need to cleanup. > > So do you want to queue these on top of the "massive" in 'next', not > directly on 'master'? If they apply on master, master. But I'm confused, are the massive changes not going to graduate to master? Because if not, I should cherry-pick the safest changes, as there's a lot of good stuff there. -- Felipe Contreras -- 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 0/2] remote-bzr: couple of fixes
Felipe Contreras writes: > On Sun, May 5, 2013 at 1:33 PM, Junio C Hamano wrote: >> Felipe Contreras writes: >> >>> The previous version had an indentation bug (did I mention I hate python?). >>> >>> A few fixes to be applied on top of the massive changes already queued. >>> Nothing >>> major. >> >> [2/2] may not matter much in the context of my tree (people would >> use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3 >> from my tree ;-), > > Maybe, but if even if they have the latest git, pushing a tag will > fail miserably, and with the patch it would fail nicely :) > >> but [1/2] sounds like it is a good thing to have >> in 1.8.3 (not "on top of that 'massive' series"). >> >> Assuming the "otherwise some version of bzr might barf" problem is >> that repo.generate_revision_history() in those versions may not >> apply str() to its first parameter and the caller is expected to >> pass a string there, or something? > > No, there's no change to repo.generate_revision_history(), because we > already convert the elements of the array to strings, it's the other > callers of Marks::to_rev() that see a change, namely code that pushes > to a remote, I think. > > And BTW, they are already strings, but unicode strings, because they > come from a json file, somehow bazaar doesn't like that, but it works > fine in my machine without the patch. Shrugs. > > Also, the emacs developers seem to be fine with all these changes, > there's only one patch pending that I need to cleanup. So do you want to queue these on top of the "massive" in 'next', not directly on 'master'? -- 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 0/2] remote-bzr: couple of fixes
On Sun, May 5, 2013 at 1:33 PM, Junio C Hamano wrote: > Felipe Contreras writes: > >> The previous version had an indentation bug (did I mention I hate python?). >> >> A few fixes to be applied on top of the massive changes already queued. >> Nothing >> major. > > [2/2] may not matter much in the context of my tree (people would > use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3 > from my tree ;-), Maybe, but if even if they have the latest git, pushing a tag will fail miserably, and with the patch it would fail nicely :) > but [1/2] sounds like it is a good thing to have > in 1.8.3 (not "on top of that 'massive' series"). > > Assuming the "otherwise some version of bzr might barf" problem is > that repo.generate_revision_history() in those versions may not > apply str() to its first parameter and the caller is expected to > pass a string there, or something? No, there's no change to repo.generate_revision_history(), because we already convert the elements of the array to strings, it's the other callers of Marks::to_rev() that see a change, namely code that pushes to a remote, I think. And BTW, they are already strings, but unicode strings, because they come from a json file, somehow bazaar doesn't like that, but it works fine in my machine without the patch. Shrugs. Also, the emacs developers seem to be fine with all these changes, there's only one patch pending that I need to cleanup. Cheers. -- Felipe Contreras -- 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 0/2] remote-bzr: couple of fixes
Felipe Contreras writes: > The previous version had an indentation bug (did I mention I hate python?). > > A few fixes to be applied on top of the massive changes already queued. > Nothing > major. [2/2] may not matter much in the context of my tree (people would use post 1.8.2 fast-export if they are using remote-bzr from 1.8.3 from my tree ;-), but [1/2] sounds like it is a good thing to have in 1.8.3 (not "on top of that 'massive' series"). Assuming the "otherwise some version of bzr might barf" problem is that repo.generate_revision_history() in those versions may not apply str() to its first parameter and the caller is expected to pass a string there, or something? Thanks. > > Felipe Contreras (2): > remote-bzr: convert all unicode keys to str > remote-bzr: avoid bad refs > > contrib/remote-helpers/git-remote-bzr | 42 > +-- > 1 file changed, 25 insertions(+), 17 deletions(-) -- 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
