Skip Menu |
Report information
Id: 133744
Status: resolved
Priority: 0/
Queue: perl5

Owner: jkeenan <jkeenan [at] cpan.org>
Requestors: jkeenan [at] pobox.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)



Date: Sun, 30 Dec 2018 10:49:27 -0500
Subject: sv.c: Eliminate LGTM warning Comparison result is always the same
From: James E Keenan <jkeenan [...] pobox.com>
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 781b
In RT 133703, I attempted to address the result of a finding from the lgtm.com analysis of the source code in sv.c. Links: https://lgtm.com/rules/2154840804/ lgtm rule for comparisons always giving same result: https://lgtm.com/rules/2154840804/ That ticket stalled when it became apparent that we didn't have a sufficiently clear understanding of the so-called "paragraph mode" ##### $/ = ""; ##### ... and that that functionality was undertested. We provided better documentation and more tests in RT 133722. So now we should be in a better position to apply a patch along the lines suggested by Dave Mitchell in 133703. Please review the patch attached, which is smoking in this branch: smoke-me/jkeenan/davem/sv-c-comparison-same Thank you very much. Jim Keenan
Download perl_V.txt
text/plain 567b

Message body is not shown because sender requested not to inline it.

Message body is not shown because sender requested not to inline it.

To: perl5-porters [...] perl.org
Subject: Re: [perl #133744] sv.c: Eliminate LGTM warning Comparison result is always the same
Date: Wed, 2 Jan 2019 08:57:27 +0000
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 549b
On Sun, Dec 30, 2018 at 07:49:34AM -0800, James E Keenan (via RT) wrote: Show quoted text
> ... and that that functionality was undertested. We provided better > documentation and more tests in RT 133722. So now we should be in a > better position to apply a patch along the lines suggested by Dave > Mitchell in 133703. > > Please review the patch attached, which is smoking in this branch: > > smoke-me/jkeenan/davem/sv-c-comparison-same
Can we ditch the gratuitous whitespace changes on otherwise unaffected lines, please! -- Monto Blanco... scorchio!
From: Sawyer X <xsawyerx [...] gmail.com>
To: Dave Mitchell <davem [...] iabyn.com>, perl5-porters [...] perl.org
Subject: Re: [perl #133744] sv.c: Eliminate LGTM warning Comparison result is always the same
Date: Sat, 5 Jan 2019 18:09:35 +0200

On 1/2/19 10:57 AM, Dave Mitchell wrote:
Show quoted text
On Sun, Dec 30, 2018 at 07:49:34AM -0800, James E Keenan (via RT) wrote:
... and that that functionality was undertested.  We provided better 
documentation and more tests in RT 133722.  So now we should be in a 
better position to apply a patch along the lines suggested by Dave 
Mitchell in 133703.

Please review the patch attached, which is smoking in this branch:

smoke-me/jkeenan/davem/sv-c-comparison-same
Can we ditch the gratuitous whitespace changes on otherwise unaffected
lines, please!


Thank you, Jim, for providing a necessary patch with a fix.

(In the future, commits for whitespace are best done separately to reduce the workload when reviewing them. It makes them easier to apply, rebase, and merge.)

Dave, the whitespace is not your enemy. (Well, maybe it is... but Jim isn't!) :)

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 163b
On Sun, 30 Dec 2018 07:49:34 -0800, jkeenan@pobox.com wrote: Show quoted text
> Please review the patch attached, which is smoking in this branch:
It looks reasonable to me. Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 339b
On Tue, 29 Jan 2019 23:30:32 GMT, tonyc wrote: Show quoted text
> On Sun, 30 Dec 2018 07:49:34 -0800, jkeenan@pobox.com wrote:
> > Please review the patch attached, which is smoking in this branch:
> > It looks reasonable to me. > > Tony
Thanks. Merged to blead in commit e8db349f5c61708301fd5463e49bfe95c448dd6d. -- James E Keenan (jkeenan@cpan.org)
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.30.0, this and 160 other issues have been resolved. Perl 5.30.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.30.0 If you find that the problem persists, feel free to reopen this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org