Re: [PATCH 3/5] git-cherry-pick: Add ignore-if-made-empty option [v2]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Neil Horman <nhorman@xxxxxxxxxxxxx> 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?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]