From: Thomas Rast <hidden> Date: 2016-06-15 22:57:29
Felipe Contreras [off-list ref] writes:
On Wed, May 29, 2013 at 3:09 AM, Thomas Rast [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
quoted
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.
I think all the points in this discussion have been made in the lengthy
thread about remote-{bzr,hg} already. So much so in fact that your
continuing defiance of the project standards, knowing that they will
elicit exactly this response, amounts to trolling.
So until this changes, my $0.02 is a blanket NAK and a refusal to spend
my time reviewing.
--
Thomas Rast
trast@{inf,student}.ethz.ch
From: Felipe Contreras <hidden> Date: 2016-06-15 22:57:29
On Wed, May 29, 2013 at 3:40 AM, Thomas Rast [off-list ref] wrote:
Felipe Contreras [off-list ref] writes:
quoted
On Wed, May 29, 2013 at 3:09 AM, Thomas Rast [off-list ref] wrote:
quoted
Felipe Contreras [off-list ref] writes:
quoted
quoted
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.
If you care so much about "*the lines of existing note copying*", why
don't you implement tests that check for those? This not only would
prevent that some shmuck who is not well versed in the tradition of
"the lines of existing note copying" from breaking that tradition,
which is precisely what tests are for.
If this was done, we wouldn't be "time-wasting" here.
This patch makes 'git cherry-pick' pass all the tests that 'git
rebase' passes. Period.
Even if it was against "the lines of existing note copying", it's much
better than the current situation, which is to do ABSOLUTELY NOTHING.
If you were a productive person, you would take this patch and
implement the code that makes it align to "the lines of existing note
copying" that you care so much about, which should be easy, if the
note framework was properly implement, but you won't.
Why isn't this implemented?
void copy_notes_for_rewrite(const char *rewrite_cmd, struct rewritten *list);
If there was such a thing, other clients wouldn't need to implement
their own methods of copying notes.
But you didn't implement that, did you?
You are punishing me because the notes framework is lacking, and
because the testing framework is missing what you think is the proper
behavior.
Strike that, you are punishing the users.
--
Felipe Contreras
So until this changes, my $0.02 is a blanket NAK and a refusal to spend
my time reviewing.
Then don't review the damn thing. With Felipe, I have the following
rule of thumb: make some concrete suggestions and forget about
follow-ups. He's not going to accept any general guidelines, unless
you're quoting Documentation/SubmittingPatches (and even then, it's
subject to interpretation); so provide a commit message and hope that
either he or Junio will use it. There is no guarantee that he will
take any of your suggestions, no matter how sensible you think they
might be. However, he is a productive programmer, and submits fixes
to real issues. He's stubborn, and we can't do much to change that:
just learn to work with him. I'm disappointed that I have to point
this out: haven't you learnt anything from previous discussions with
him?
Felipe, I suggest you put this in your commit message:
This patch implements --copy-notes for 'git cherry-pick' so it can
copy notes in the same way that 'git rebase' does.
That is, if it's not too much trouble.
Stop this back-and-fourth nonsense, both of you. It's degrading the
community, and hitting everyone's inboxes with garbage.
From: Felipe Contreras <hidden> Date: 2016-06-15 22:57:29
On Wed, May 29, 2013 at 7:09 AM, Ramkumar Ramachandra
[off-list ref] wrote:
Thomas Rast wrote:
quoted
So until this changes, my $0.02 is a blanket NAK and a refusal to spend
my time reviewing.
Then don't review the damn thing. With Felipe, I have the following
rule of thumb: make some concrete suggestions and forget about
follow-ups.
He didn't make any suggestions.
He's not going to accept any general guidelines, unless
you're quoting Documentation/SubmittingPatches (and even then, it's
subject to interpretation); so provide a commit message and hope that
either he or Junio will use it. There is no guarantee that he will
take any of your suggestions, no matter how sensible you think they
might be.
This is bullshit.
Let's look at some of the suggestions you have made:
== git-related ==
s/l/line/?
I said fine, and I implemented it.
Still no -CCC?
I said I forgot, and I implemented it.
What you are really complaining about is that I don't agree with
*every* single suggestion you make. And since you made them, they must
be sensible, and single I don't agree with you, I must not be
sensible, is that right?
And stop bringing irrelevant garbage to this discussion. The
discussion about the coding-style not about guidelines, because there
is no guideline for that open parenthesis, ad obviated by the fact
that there's over *FIVE HUNDRED* instances where it's not aligned that
way.
Nobody is denying the notes.rewritten.* guideline here, I didn't
because that is *actually* a guideline. So your comment about
guidelines is an irrelevant straw man.
However, he is a productive programmer, and submits fixes
to real issues. He's stubborn, and we can't do much to change that:
just learn to work with him. I'm disappointed that I have to point
this out: haven't you learnt anything from previous discussions with
him?
Felipe, I suggest you put this in your commit message:
This patch implements --copy-notes for 'git cherry-pick' so it can
copy notes in the same way that 'git rebase' does.
That is, if it's not too much trouble.
Stop this back-and-fourth nonsense, both of you. It's degrading the
community, and hitting everyone's inboxes with garbage.
Thanks.
But let's take a step backwards, what are we trying to achieve here?
We are trying to improve Git, and the indisputable fact is that 'git
cherry-pick' is missing a way to copy the notes.
It's indisputable that this patch implements that, and I did it by
following existing code, and by running the whole test suit for 'git
rebase'. I've done my job already.
Thomas Rast doesn't like the way this is implemented, and nothing in
the commit message would change that.
This is was a sensible community, you would stop ganging up on me,
Thomas Rast would implement copy_notes_for_rewrite(), and add tests
for other commits (git am, git rebase) to check that the functionality
he claims to be so worried about is working properly.
And this was a sensible community you wouldn't complain about me
choosing how to spend my time however I see fit.
I did some work, I sent a patch, Thomas Rast has some issues, I'm not
interested enough in this patch to investigate them, I work on
something else. What's wrong with that?
Eventually I might come back to this patch, and eventually I might
implement copy_notes_for_rewrite, and I might implement the tests that
check for the behavior that I missed, if nobody beats me to it, which
is usually the case, but I think Thomas should put his personal issues
aside, put his money where his mouth is, and implement it himself.
There's nothing wrong with me choosing how best to spend my time. Really.
--
Felipe Contreras
What you are really complaining about is that I don't agree with
*every* single suggestion you make. And since you made them, they must
be sensible, and single I don't agree with you, I must not be
sensible, is that right?
Oh, I have no problems: I reviewed git-related, and it resulted in
massive simplifications and improvements; those threads didn't run
wild. I was mainly criticizing Thomas' review style (when it comes to
reviewing your patches in particular), and was prodding him to "make
concrete suggestions" in a one-email review, and leave it at that.
What started out as a pointer to the guidelines:
Thomas Rast wrote:
We've been over this already:
The body should provide a meaningful commit message, which:
has resulted in this thread running wild.
There's nothing wrong with me choosing how best to spend my time. Really.
Ofcourse you are. You have arguably spent it very productively
solving a lot of user issues (especially remote-bzr).
Personally, I try to do the minimum amount of boring work required to
make sure that a good series gets in. Sometimes this is a little high
(for a recent example, see my pickaxe-doc series). The result being
that I just won't work on documentation in the future because doing
iterations is so piss boring: the git community needs to recognize
this problem and make amends.
From: Felipe Contreras <hidden> Date: 2016-06-15 22:57:29
On Wed, May 29, 2013 at 8:48 AM, Ramkumar Ramachandra
[off-list ref] wrote:
Felipe Contreras wrote:
quoted
There's nothing wrong with me choosing how best to spend my time. Really.
Ofcourse you are. You have arguably spent it very productively
solving a lot of user issues (especially remote-bzr).
Personally, I try to do the minimum amount of boring work required to
make sure that a good series gets in. Sometimes this is a little high
(for a recent example, see my pickaxe-doc series). The result being
that I just won't work on documentation in the future because doing
iterations is so piss boring: the git community needs to recognize
this problem and make amends.
I don't mind doing as many iterations as it takes, as long as it's
about meaningful issues.
I don't particularly enjoy, but I'm OK with discussing non-meaningful issues.
What I'm not OK with is disagreements that end up breaking the
communication, specially when a crystal-clear case has been made
(IMO), and the patch goes to limbo for no reason. That I think is a
real problem.
For this particular patch, I don't care if it goes in at the moment. I
have something big on the pipeline, and I would rather drop this than
loose penguin points, although I probably don't have any left.
Cheers.
--
Felipe Contreras