develooper Front page | perl.perl5.porters | Postings from February 2018

Re: [perl #131046] [PATCH] Carp: Do not crash when reading @DB::args

Thread Previous | Thread Next
February 24, 2018 00:59
Re: [perl #131046] [PATCH] Carp: Do not crash when reading @DB::args
Message ID:
On 24 Feb 2018 08:01, "Zefram" <> wrote:

demerphq wrote:
>Patch pushed as 4764858cb80e76fdba33cc1b3be8fcdef26df754

There are a couple of problems with this patch, going beyond my
distaste for the inadequacy of the code change.

Zefram, that is an uncalled for comment and beneath you.


, the commit
message contains exactly the kind of misconception that I argued against.

You had *weeks* to point this out and did not.

Rather than describing the effect as partial, the message says "This patch
safely iterates all elements", "This prevent crashing perl", and "It [is]
... workaround for Carp module".  (Bad grammar all in the actual message.)
This is describing the patch as having an effectiveness that it doesn't.

You had weeks to point this out and did not.

Secondly, the newly added test is predicated on that very same mythical
effectiveness.  It sets up a stack refcounting failure, at which point
behaviour is undefined, whether the Carp patch is applied or not.
This is not a correct test: it can fail without anything getting more
wrong than it already is.

You had weeks to point this out and did not.

The last argument advanced in favour of this patch, unlike many that
had preceded it, was supposedly based on a correct understanding of its
limitations.  I didn't argue against that, because my greater objection
is about the proposed patch being misunderstood and oversold.

Again, you had weeks to comment on the patch and did not.

What is the point of pushing a patch for review if people like you wait to
state your feedback until it has been merged.

Where it's
correctly understood, it's more a matter of opinion whether it's useful.

No it's not. It is clearly useful in some cases.

But the patch as applied doesn't seem to have taken that correct
understanding into account.  It is firmly based on the misconception,
and I seem to have been fooled into dropping my objections by some talk
that doesn't actually reflect the totality of the patch

From my point of view you didn't drop your objections, you disengaged from
the process when challenged. If you wish to take a pasdive-aggressive
stance when people push back on your view then this kind of thing is the
inevitable result.

Again, you had weeks to raise any and all of these points but you chose not
to respond to my last mail.

Bonus problem: the patch desynchronises the version numbers of Carp and
Carp::Heavy, giving the former an unjustified underscored version.

That is a mistake, which I honestly thought I had fixed. Another patch was
made to Carp while this was "cooking" which did a full version bump, which
lead to conflicts, I then did a second patch which also did a version bump.
I used underscored versions because I didn't and don't think that core
module changes only present in a blead interim releases should result in
version number churn.

I would really appreciate it if you could take a step back, and perhaps
climb down from your pedestal when you communicate with me. Throwing around
terms like "unjustified" in this context is unhelpful and comes off as
unduly arrogant, and frankly just irritated me. We all know you are one of
the smartest and best hackers on this list, but you really don't need to
take that kind of attitude with people who are just trying to do their best
to move things forward. There are a million ways you could have started a
discussion about the version number changes without offending me.

Clearly the new test needs to be deleted.

No, that isn't clear.

  I don't know what to do about
the false commit message, though.

Again, your choice of words here, especially given the amount of time that
you have had to raise these concerns is really irritating. Even a minor
change like "misleading" would make me far more likely to engage you
seriously on this point and far less likely to be irritated.


 message doesn't reference this
ticket, so putting a correction here doesn't serve as an annotation.
I'm tempted to revert the commit,

That would be the height of rudeness and most inappropriate. You had weeks
to write this mail and chose not to. That duration means to me you have
lost any right to take unilateral action.

If anybody is going to do anything like that it should be me.


the basis that the code change can
later be applied again with a correct message.  Then at least git blame
would point at something correct.

You had weeks to raise these points and chose to stay silent. Next time try
a different strategy. This pasdive-aggressive say nothing until the patch
is merged and then ruthlessly criticise what has been merged is not how we
are supposed to work together.

I will do something about this later.


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About