Re: What's cooking in git.git (Dec 2012, #03; Wed, 12)

On Thu, Dec 13, 2012 at 6:04 AM, Max Horn [off-list ref] wrote:

On 13.12.2012, at 11:08, Felipe Contreras wrote:

quoted

On Thu, Dec 13, 2012 at 2:11 AM, Junio C Hamano [off-list ref] wrote:

quoted

Felipe Contreras [off-list ref] writes:

quoted

quoted

quoted

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.

quoted

quoted

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