git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Phil Hord <phil.hord@gmail.com>
Subject: Re: [PATCH 3/5] git-cherry-pick: Add ignore-if-made-empty option [v2]
Date: Thu, 05 Apr 2012 18:20:24 -0700	[thread overview]
Message-ID: <7vvcld3bc7.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120405234527.GB8654@hmsreliant.think-freely.org> (Neil Horman's message of "Thu, 5 Apr 2012 19:45:27 -0400")

Neil Horman <nhorman@tuxdriver.com> writes:

> On Thu, Apr 05, 2012 at 02:01:43PM -0700, Junio C Hamano wrote:
>
>> I am starting to wonder if it is worth spending time on careful reviewing,
>> or it would be sufficient to give a cursory review quickly to give you
>> more time to polish your re-roll.
>> 
> I'm not sure what you think is so egregious about this changeset, but if
> you have a specific problem, please let me know.

There is no insult involved. I just didn't know where to start, because
the series was littered with many issues from high level design (e.g. does
the command line interface and API addition make sense?) to low level
styles (e.g. does the new code imitate the style of the existing code
around it?), and in between (e.g. pipe2() is never used in the codebase
without this patch. Is it portable enough?). It was clear that it needed a
lot more work to lose a WIP label (the quality standard in this project is
slightly higher than "the end result seems to compile--let's ship it").

In other words, I was simply being honest.

> We all make errors, thats why
> we review work like this.  All your comment above does is toss a purposeless
> insult into the conversation. 

Making mistakes is one thing.  Sending a series that is not sufficiently
proofread is a completely different matter.  The review process is not a
replacement for your own proofreading.  It comes after that.

If you did proofread the patch [3/5], you would have noticed that the fix
you made to the documentation is a fix for patch [2/5]. You are in much
better position than I or other reviewers to notice it---after all, it is
your addition. The same for the typo in the mysteriously named function.

How else do you expect me to react to such a series?

  reply	other threads:[~2012-04-06  1:20 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-30 19:48 [PATCH 0/4] Enhance git-rebases flexibiilty in handling empty commits Neil Horman
2012-03-30 19:48 ` [PATCH 1/4] git-cherry-pick: add keep-empty option Neil Horman
2012-03-30 20:34   ` Junio C Hamano
2012-03-30 21:15   ` Jeff King
2012-03-31 12:57   ` Neil Horman
2012-03-30 19:48 ` [PATCH 2/4] git-rebase: add keep_empty flag Neil Horman
2012-03-30 20:43   ` Junio C Hamano
2012-03-31 12:59     ` Neil Horman
2012-03-30 19:48 ` [PATCH 3/4] git-commit-am: Allow automatic rebasing to preserve empty commits Neil Horman
2012-03-30 20:46   ` Junio C Hamano
2012-03-31 13:01     ` Neil Horman
2012-03-30 20:47   ` Junio C Hamano
2012-03-31 13:03     ` Neil Horman
2012-03-30 19:48 ` [PATCH 4/4] git-commit-interactive: Allow " Neil Horman
2012-03-30 20:59   ` Junio C Hamano
2012-03-31 13:11     ` Neil Horman
2012-03-30 20:32 ` [PATCH 0/4] Enhance git-rebases flexibiilty in handling " Junio C Hamano
2012-04-05 19:39 ` [PATCH 0/5] Enhance git-rebases flexibiilty handling empty commits [v2] Neil Horman
2012-04-05 19:39   ` [PATCH 1/5] argv-array: Add argv_array_pop function [v2] Neil Horman
2012-04-05 20:12     ` Junio C Hamano
2012-04-05 23:24       ` Neil Horman
2012-04-06  0:12         ` Neil Horman
2012-04-06  0:19         ` Jeff King
2012-04-06  1:12           ` Neil Horman
2012-04-06  1:20         ` Junio C Hamano
2012-04-06  2:20           ` Jeff King
2012-04-06  4:19             ` Cc tags in the commit message (Re: [PATCH 1/5] argv-array: Add argv_array_pop function [v2]) Jonathan Nieder
2012-04-06  6:55             ` [PATCH 1/5] argv-array: Add argv_array_pop function [v2] Junio C Hamano
2012-04-06  7:33               ` Jeff King
2012-04-06 16:49                 ` Junio C Hamano
2012-04-06 18:02             ` Neil Horman
2012-04-05 19:39   ` [PATCH 2/5] git-cherry-pick: add allow-empty option [v2] Neil Horman
2012-04-05 19:39   ` [PATCH 3/5] git-cherry-pick: Add ignore-if-made-empty " Neil Horman
2012-04-05 21:01     ` Junio C Hamano
2012-04-05 23:45       ` Neil Horman
2012-04-06  1:20         ` Junio C Hamano [this message]
2012-04-06 18:06       ` Neil Horman
2012-04-06 18:30     ` Johannes Sixt
2012-04-05 19:39   ` [PATCH 4/5] git-cherry-pick: Add test to validate new options [v2] Neil Horman
2012-04-05 19:39   ` [PATCH 5/5] git-rebase: add keep_empty flag [v2] Neil Horman
2012-04-10 15:47 ` [PATCH v3 0/4] Enhance git-rebases flexibiilty in handling empty commits Neil Horman
2012-04-10 15:47   ` [PATCH v3 1/4] git-cherry-pick: add allow-empty option Neil Horman
2012-04-10 16:45     ` Junio C Hamano
2012-04-10 18:13       ` Neil Horman
2012-04-10 19:32         ` Junio C Hamano
2012-04-10 20:00           ` Neil Horman
2012-04-10 20:32             ` Junio C Hamano
2012-04-10 20:39               ` Neil Horman
2012-04-10 21:09                 ` Junio C Hamano
2012-04-11  0:44                   ` Neil Horman
2012-04-11 16:52                     ` Junio C Hamano
2012-04-11 18:29                       ` Neil Horman
2012-04-11 18:50                         ` Junio C Hamano
2012-04-11 18:56                           ` Neil Horman
2012-04-10 15:47   ` [PATCH v3 2/4] git-cherry-pick: Add keep-redundant-commits option Neil Horman
2012-04-10 17:04     ` Junio C Hamano
2012-04-10 18:25       ` Neil Horman
2012-04-10 15:47   ` [PATCH v3 3/4] git-cherry-pick: Add test to validate new options Neil Horman
2012-04-10 15:47   ` [PATCH v3 4/4] git-rebase: add keep_empty flag Neil Horman
2012-04-13 18:45 ` [PATCH v5 0/4]Enhance git-rebases flexibiilty in handling empty commits Neil Horman
2012-04-13 18:45   ` [PATCH v5 1/4] git-cherry-pick: add allow-empty option Neil Horman
2012-04-13 18:45   ` [PATCH v5 2/4] git-cherry-pick: Add keep-redundant-commits option Neil Horman
2012-04-15 10:42     ` Clemens Buchacher
2012-04-16 15:38       ` Neil Horman
2012-04-16 22:10         ` Clemens Buchacher
2012-04-17 10:43           ` Neil Horman
2012-04-17 15:42           ` Junio C Hamano
2012-04-17 21:37             ` Clemens Buchacher
2012-04-18 10:41               ` Neil Horman
2012-04-13 18:45   ` [PATCH v5 3/4] git-cherry-pick: Add test to validate new options Neil Horman
2012-04-15  9:39     ` Clemens Buchacher
2012-04-16 11:04       ` Neil Horman
2012-04-16 16:14       ` Neil Horman
2012-04-16 16:35         ` Junio C Hamano
2012-04-16 16:50           ` Neil Horman
2012-04-16 21:42             ` Clemens Buchacher
2012-04-17 10:56               ` Neil Horman
2012-04-17 21:38                 ` Clemens Buchacher
2012-04-18 10:48                   ` Neil Horman
2012-04-18 18:34                     ` Clemens Buchacher
2012-04-13 18:45   ` [PATCH v5 4/4] git-rebase: add keep_empty flag Neil Horman
2012-04-15  9:33     ` Clemens Buchacher
2012-04-16 16:46       ` Neil Horman
2012-04-17 18:20 ` [PATCH v6 0/4] Enhance git-rebases flexibiilty in handling empty commits Neil Horman
2012-04-17 18:20   ` [PATCH v6 1/4] git-cherry-pick: add allow-empty option Neil Horman
2012-04-17 18:20   ` [PATCH v6 2/4] git-cherry-pick: Add keep-redundant-commits option Neil Horman
2012-04-17 21:45     ` Clemens Buchacher
2012-04-18 10:49       ` Neil Horman
2012-04-17 18:20   ` [PATCH v6 3/4] git-cherry-pick: Add test to validate new options Neil Horman
2012-04-17 18:20   ` [PATCH v6 4/4] git-rebase: add keep_empty flag Neil Horman
2012-04-17 21:47     ` Clemens Buchacher
2012-04-18 10:50       ` Neil Horman
2012-04-18 22:58     ` Junio C Hamano
2012-04-19 13:08       ` Neil Horman
2012-04-19 17:53         ` Junio C Hamano
2012-04-18 19:17 ` [PATCH v7 0/4] Enhance git-rebases flexibiilty in handling empty commits Neil Horman
2012-04-18 19:17   ` [PATCH v7 1/4] git-cherry-pick: add allow-empty option Neil Horman
2012-04-18 19:17   ` [PATCH v7 2/4] git-cherry-pick: Add keep-redundant-commits option Neil Horman
2012-04-18 22:59     ` Junio C Hamano
2012-04-18 19:17   ` [PATCH v7 3/4] git-cherry-pick: Add test to validate new options Neil Horman
2012-04-18 19:18   ` [PATCH v7 4/4] git-rebase: add keep_empty flag Neil Horman
2012-04-19 10:02     ` Zbigniew Jędrzejewski-Szmek
2012-04-19 11:49       ` Thomas Rast
2012-04-19 12:19         ` Zbigniew Jędrzejewski-Szmek
2012-04-19 13:12           ` Neil Horman
2012-04-19 18:59         ` Junio C Hamano
2012-04-19 19:05           ` Junio C Hamano
2012-04-20 10:35             ` Neil Horman
2012-04-20 14:36 ` [PATCH v8 0/4] Enhance git-rebases flexibiilty in handling empty commits Neil Horman
2012-04-20 14:36   ` [PATCH v8 1/4] git-cherry-pick: add allow-empty option Neil Horman
2012-04-20 14:36   ` [PATCH v8 2/4] git-cherry-pick: Add keep-redundant-commits option Neil Horman
2012-04-20 19:21     ` Junio C Hamano
2012-04-20 19:56       ` Neil Horman
2012-04-20 14:36   ` [PATCH v8 3/4] git-cherry-pick: Add test to validate new options Neil Horman
2012-04-20 14:36   ` [PATCH v8 4/4] git-rebase: add keep_empty flag Neil Horman
2012-04-25  1:50     ` Junio C Hamano
2012-04-25 10:38       ` Neil Horman
2012-07-18  6:20     ` Martin von Zweigbergk
2012-07-18  7:10       ` Johannes Sixt
2012-07-18  7:16         ` Martin von Zweigbergk
2012-07-18 12:17         ` Neil Horman

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=7vvcld3bc7.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=peff@peff.net \
    --cc=phil.hord@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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).