On Thu, Dec 13, 2012 at 6:04 AM, Max Horn <post...@quendi.de> wrote:
> On 13.12.2012, at 11:08, Felipe Contreras wrote:
>> On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Felipe Contreras <felipe.contre...@gmail.com> 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:

        import bzrlib
        import bzrlib.plugin
        import bzrlib.plugins.fastimport

        if ! "$PYTHON_PATH" -c "$cmd"; then
                echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 
                skip_all='skipping remote-bzr tests; bzr-fastimport not 

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.


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

Reply via email to