On Thu, Dec 13, 2012 at 6:04 AM, Max Horn <postbox@xxxxxxxxx> wrote: > > On 13.12.2012, at 11:08, Felipe Contreras wrote: > >> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >>> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: >> >>>>> New remote helper for bzr (v3). With minor fixes, this may be ready >>>>> for 'next'. >>>> >>>> What minor fixes? >>> >>> Lookng at the above (fixup), $gmane/210744 comes to mind >> >> That doesn't matter. The code and the tests would work just fine. > > > It doesn't matter? I find that statement hard to align with what the maintainer of git, and thus the person who decides whether your patch series gets merged or not, wrote just above? In fact, it seems to me that what Junio said matters a great deal... So you think Junio knows more about remote-bzr than I do? I repeat; it doesn't affect the tests, it doesn't affect the code, it doesn't cause any problem. remote-bzr could be merged today, in fact, it could have been merged a month ago. You don't trust me? Here, look: cmd=<<EOF import bzrlib bzrlib.initialize() import bzrlib.plugin bzrlib.plugin.load_plugins() import bzrlib.plugins.fastimport EOF if ! "$PYTHON_PATH" -c "$cmd"; then echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2 skip_all='skipping remote-bzr tests; bzr-fastimport not available' test_done fi All this code is a no-op, because, as Junio pointed out, cmd is null. How is that a problem? It's not. The first version of remote-bzr relied on the bazaar fastimport plug-in, so this check was needed, in case you had bazaar, but not this particular plug-in, but today remote-bzr doesn't need this plug-in, so this chunk of code should be removed. The fact that this code does nothing (because python -c '' does nothing) is *not a problem*. In fact, even if that code failed 100% of the time, it wouldn't hurt anybody, because 'make -C t' would work, everything would work, the only thing that would fail is 'make -C contrib/remote-helpers/ test-bzr', which very very few people would consider a problem. But it doesn't fail, it works. Who benefits by delaying the merging of this code? Nobody. Who gets hurt? The users, of course. > This is a very strange attitude... > > In another email, you complained about nobody reviewing your patches respectively nobody voicing any constructive criticism. Yet Junio did just that, and again in $gmane/210745 -- and you replied to neither, and acted on neither (not even by refuting the points brought up), and now summarily dismiss them as irrelevant. I find that quite disturbing :-(. I didn't say it was irrelevant, it should be fixed, but Junio said "With minor fixes, this may be ready for 'next'." which is no true IMO, it's ready *now*, it was ready one month ago. For 'next', this problem doesn't matter. The feedback is appreciated, but delaying the merging of this code for no reason makes little sense to me. Junio, of course, can do whatever he wants. The removal of this no-op code can wait, or it can be done on top of v3, there's no need for re-roll, and Junio already complained about the v3 re-roll. And I didn't act because I was on vacations, git development is not my only priority. And even if I had time, I don't see why I should prioritize this fix, it's not important, the code is ready. >>> but there may be others. It is the responsibility of a contributor to keep >>> track of review comments others give to his or her patches and >>> reroll them, so I do not recall every minor details, sorry. >> >> There is nothing that prevents remote-bzr from being merged. > > Well, I think that is up to Junio to decide in the end, though :-). He wrote No. He can decide if the code gets merged, but he is not the voice of truth. Nothing prevents him from merging the code, except himself. There is no known issue with the code, that is a true fact. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html