git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Thomas Rast <trast@inf.ethz.ch>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
	Ramkumar Ramachandra <artagnon@gmail.com>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [PATCH 1/3] cherry-pick: add support to copy notes
Date: Wed, 29 May 2013 06:56:54 -0500	[thread overview]
Message-ID: <CAMP44s2de-kNnxQvLNE5QrDFYMX=Omyu61+J=Dbxp0of3iYGxg@mail.gmail.com> (raw)
In-Reply-To: <874ndmqate.fsf@linux-k42r.v.cablecom.net>

On Wed, May 29, 2013 at 6:34 AM, Thomas Rast <trast@inf.ethz.ch> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Wed, May 29, 2013 at 3:40 AM, Thomas Rast <trast@inf.ethz.ch> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> On Wed, May 29, 2013 at 3:09 AM, Thomas Rast <trast@inf.ethz.ch> wrote:
>>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>>
>>>>>> Feel free to implement that. I'm just interested in 'git cherry-pick' being
>>>>>> usable for 'git rebase' purposes.
>>>>>
>>>>> Which would have been obvious to all but the most casual readers, eh?
>>>>
>>>> My motivations are irrelevant, the patch is good as it is.
>>>
>>> You fooled both Junio (AFAICT anyway) and me, who both reviewed the
>>> patch under the assumption that it implements note copying *along the
>>> lines of existing note copying*.  This proved to be a wrong, and
>>> time-wasting, assumption.
>>
>> Whatever arbitrary rules you are talking about, they are not codified in tests.
>
> Tests or code don't have a thing to do with it.  This is about how you
> are presenting your changes to the rest of the git community.  As
> evidenced above, said presentation is not clear enough to communicate
> your goals to at least two experienced git developers (if I may say so
> myself).  How are we supposed to review a change if it is not even clear
> what goal it satisfies?

My goals are irrelevant. This patch is good.

It implements a missing feature, if you don't like how it is implemented it:

a) Implement the code in the note framework that does it properly so
other people can just call that.

b) Implement the tests for other commands that check that the behavior
you talk about does happen indeed.

c) Implement it yourself

This has nothing to do with the way I presented the patch. I presented
the patch as I thought it was; implementing the note copying as it was
intended. Now you came along and say I wasted your time because I
didn't say I implemented it wrongly, and you assume bad faith and what
not.

This wouldn't have happened because you didn't do a) or b). I realized
that by replacing 'am' with 'cherry-pick' in 'git rebase'  the notes
were not copied, according to the testing framework, so I implemented
that, and the tests pass. Now you come along and say it should
implemented some note.rewrite.command stuff, but the tests didn't
check for that, so how was I supposed to know?

And then you have the audacity to claim that *I*, the one who just
wrote a bunch of code to implement a missing feature, is wasting
*your* time, you, the one who is simply replying to an email and
shooting from the hip.

> Again: I'll be happy to review your proposed changes if and when you
> resend the series with commit messages.

I won't, I'll keep working on my actual objective.

Plus, this patch does have a commit message, and the commit message
says *EXACTLY* what the patch is doing: add support to copy notes.

If *you* are interested in certain behavior, why don't you lift a
finger and do something to make sure that such behavior is easy to
implement and the test framework actually checks for that?

-- 
Felipe Contreras

  reply	other threads:[~2013-05-29 11:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-28 12:59 [PATCH 0/3] cherry-pick: improvments Felipe Contreras
2013-05-28 12:59 ` [PATCH 1/3] cherry-pick: add support to copy notes Felipe Contreras
2013-05-28 17:07   ` Junio C Hamano
2013-05-28 18:01     ` Thomas Rast
2013-05-29  2:46       ` Felipe Contreras
2013-05-29  8:09         ` Thomas Rast
2013-05-29  8:19           ` Felipe Contreras
2013-05-29  8:40             ` Thomas Rast
2013-05-29 11:18               ` Felipe Contreras
2013-05-29 11:34                 ` Thomas Rast
2013-05-29 11:56                   ` Felipe Contreras [this message]
2013-05-29 12:09               ` Ramkumar Ramachandra
2013-05-29 13:18                 ` Felipe Contreras
2013-05-29 13:48                   ` Ramkumar Ramachandra
2013-05-29 14:01                     ` Felipe Contreras
2013-05-29  2:41     ` Felipe Contreras
2013-05-28 12:59 ` [PATCH 2/3] revert/cherry-pick: add --quiet option Felipe Contreras
2013-05-28 12:59 ` [PATCH 3/3] revert/cherry-pick: add --skip option 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CAMP44s2de-kNnxQvLNE5QrDFYMX=Omyu61+J=Dbxp0of3iYGxg@mail.gmail.com' \
    --to=felipe.contreras@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=trast@inf.ethz.ch \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).