Front page | perl.perl5.porters |
Postings from February 2018
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. Firstly, the commit message contains exactly the kind of misconception that I argued against. 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. 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. 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. Where it's correctly understood, it's more a matter of opinion whether it's useful. 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. Bonus problem: the patch desynchronises the version numbers of Carp and Carp::Heavy, giving the former an unjustified underscored version. Clearly the new test needs to be deleted. I don't know what to do about the false commit message, though. The message doesn't reference this ticket, so putting a correction here doesn't serve as an annotation. I'm tempted to revert the commit, on 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. -zeframThread Previous | Thread Next