archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <>
To: Felipe Contreras <>
Cc: Johan Herland <>,,,,,,,,
Subject: Re: [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch]
Date: Thu, 13 Jun 2013 10:24:32 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Felipe Contreras's message of "Wed, 12 Jun 2013 15:11:59 -0500")

Felipe Contreras <> writes:

> On Wed, Jun 12, 2013 at 3:02 PM, Junio C Hamano <> 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

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.


  reply	other threads:[~2013-06-13 17:24 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-08 17:29 [PATCH] build: get rid of the notion of a git library Felipe Contreras
2013-06-08 18:02 ` Ramkumar Ramachandra
2013-06-08 18:22   ` Felipe Contreras
2013-06-09 14:56     ` Ramkumar Ramachandra
2013-06-09 15:12       ` John Keeping
2013-06-09 15:40         ` Felipe Contreras
2013-06-09 16:02           ` John Keeping
2013-06-09 16:22             ` Felipe Contreras
2013-06-09 16:42               ` John Keeping
2013-06-09 17:03                 ` Ramkumar Ramachandra
2013-06-09 17:12                   ` Ramkumar Ramachandra
2013-06-09 17:13                   ` Felipe Contreras
2013-06-09 17:32                     ` John Keeping
2013-06-09 17:45                       ` Felipe Contreras
2013-06-09 16:36             ` Ramkumar Ramachandra
2013-06-09 17:30           ` Vincent van Ravesteijn
2013-06-09 17:35             ` Felipe Contreras
2013-06-10 21:45             ` Jeff King
2013-06-10 21:52               ` Felipe Contreras
2013-06-10 22:06                 ` Jeff King
2013-06-10 22:22                   ` Felipe Contreras
2013-06-10 23:05                   ` Junio C Hamano
2013-06-10 23:41                     ` Junio C Hamano
2013-06-10 23:51                       ` Felipe Contreras
2013-06-11  0:04                         ` Junio C Hamano
2013-06-11  1:53                           ` Junio C Hamano
2013-06-11  4:15                             ` Felipe Contreras
2013-06-11 17:33                               ` Junio C Hamano
2013-06-11 17:41                                 ` Felipe Contreras
2013-06-11 17:58                                   ` Junio C Hamano
2013-06-11 18:06                                     ` Felipe Contreras
2013-06-11 18:14                                       ` Linus Torvalds
2013-06-11 19:15                                         ` Felipe Contreras
2013-06-11 19:59                                         ` Junio C Hamano
2013-06-11 20:12                                           ` Felipe Contreras
2013-06-12  0:12                                           ` [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch] Johan Herland
2013-06-12  0:12                                             ` [PATCH 1/3] finish_copy_notes_for_rewrite(): Let caller provide commit message Johan Herland
2013-06-12 17:27                                               ` Junio C Hamano
2013-06-12  0:13                                             ` [PATCH 2/3] Move copy_note_for_rewrite + friends from builtin/notes.c to notes-utils.c Johan Herland
2013-06-12  0:32                                               ` Felipe Contreras
2013-06-12  7:10                                                 ` Johan Herland
2013-06-12 18:28                                                   ` Felipe Contreras
2013-06-12 19:14                                                     ` Johan Herland
2013-06-12 19:18                                                       ` Felipe Contreras
2013-06-13  6:45                                                     ` Andreas Krey
2013-06-13 13:13                                                       ` Felipe Contreras
2013-06-12 20:02                                               ` Junio C Hamano
2013-06-12  0:13                                             ` [PATCH 3/3] Move create_notes_commit() from notes-merge.c into notes-utils.c Johan Herland
2013-06-12 20:02                                             ` [PATCH 0/3] Refactor useful notes functions into notes-utils.[ch] Junio C Hamano
2013-06-12 20:11                                               ` Felipe Contreras
2013-06-13 17:24                                                 ` Junio C Hamano [this message]
2013-06-13 18:16                                                   ` Felipe Contreras
2013-06-13 18:50                                                     ` Felipe Contreras
2013-06-11 18:17                                       ` [PATCH] build: get rid of the notion of a git library Junio C Hamano
2013-06-11 19:01                                         ` Felipe Contreras
2013-06-11 19:24                                           ` Junio C Hamano
2013-06-11 19:49                                             ` Felipe Contreras
2013-06-11  4:04                           ` Felipe Contreras
2013-06-09 15:41       ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).