Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> The proposed patch was rejected on the basis that it was organized
>> the code in a wrong way.  And your patch shows how it should be
>> done.
>
> In your opinion.
>
> The fact that nobody outside of 'git' will ever use
> init_copy_notes_for_rewrite() still remains. Therefore this
> "organization" is wrong.

That is a fact?

It is your opinion on what might happen in the future.  

And you ignored external projects that may want to link with
libgit.a, and closed the door for future improvements.  Johan's
implementation has the same effect of allowing sequencer.c to call
these functions without doing so.

Anyway, I have a more important thing to say.

You sometimes identify the right problem to tackle, but often the
discussions on your patches go in a wrong direction that does not
help solving the original problem at all.  The two examples I can
immediately recall offhand are:

 (1) a possible "blame" enhancement, where gitk, that currently runs
     two passes of it to identify where each line ultimately came
     from and to identify where each line was moved to the current
     place, could ask it to learn both with a single run.

 (2) refactoring builtin/notes.c to make it possible for sequencer
     machinery can also call useful helper functions buried in it.

but I am sure other reviewers can recall other instances in the
recent past.

Your patches were wrong in both cases, but that is not an issue.  If
you are not familiar with the area you are trying to improve, it is
understandable that initial attempts may try to solve the right
problem in a wrong way.  That is perfectly normal.

That is what the patch review process is there to help.

Reviewers who are more familiar with the area (either the code flow
and data structure used in blame, or how the object files are laid
out in the source tree and the build procedure is designed to link
them to which binary) can point the contributor in a direction that
would take us to a better result in the end.  During the discussion,
it may turn out that reviewers have overlooked issues that also need
to be addressed, or there may be further adjustments needed that are
initially overlooked by everyone.  The solution to these problems is
for contributors and reviewers to _collaborate_ to come up with a
better end result, which is often different from both the original
patch and the suggestions in the initial review.

When it is your patch, however, we repeatedly saw that the review
process got derailed in the middle.

The reviewers tried to reach a good end result in the same way as
they interact with other contributors, i.e. by showing a way they
think is better, trying to make the contributor realize why it is
better by rephrasing and coming up with other examples.

This iteration takes a lot of resources, but the reviewers are
hoping that we will see a good result at the end of the review and
everybody wins. They are trying to collaborate.

If there is no will to collaborate on the contributor's end,
however, and the primary thing the contributor wants to do is to
endlessly argue, the efforts by reviewers are all wasted. We do not
get anywhere.

That is how I perceive what happens to many of your patches.  I am
sure you will say "that is your opinion", but I do not think I am
alone.  And I am also sure you will then say "majority is not always
right".

But the thing is, that majority is what writes the majority of the
code and does the majority of the reviews, so as maintainer I *do*
have to give their opinion a lot of weight, not to mention my own
opinion about how to help keep the community the most productive.

And I have to conclude that the cost of having to deal with you
outweighs the benefit the project gets out of having you around.
Therefore I have ask you to leave and not bother us anymore.

Goodbye.