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

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

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.