* git svn perl issues
@ 2011-03-23 15:52 Stephen Hemminger
2011-03-23 21:38 ` Lasse Makholm
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2011-03-23 15:52 UTC (permalink / raw)
To: git
1. The following needs to be fixed:
$ git svn clone
Use of uninitialized value $_[0] in substitution (s///) at /usr/share/perl/5.10.1/File/Basename.pm line 341.
fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line 403
2. The git-svn perl script does not follow Perl Best Practices.
If you run the perlcritic script on it, all the following warnings/errors
are generated:
$ perlcritic /usr/lib/git-core/git-svn
Code before strictures are enabled at line 2, column 10. See page 429 of PBP. (Severity: 5)
Variable declared in conditional statement at line 18, column 1. Declare variables outside of the condition. (Severity: 5)
Subroutine prototypes used at line 39, column 1. See page 194 of PBP. (Severity: 5)
Stricture disabled at line 65, column 2. See page 429 of PBP. (Severity: 5)
Variable declared in conditional statement at line 287, column 1. Declare variables outside of the condition. (Severity: 5)
Variable declared in conditional statement at line 533, column 2. Declare variables outside of the condition. (Severity: 5)
Bareword file handle opened at line 851, column 3. See pages 202,204 of PBP. (Severity: 5)
Bareword file handle opened at line 1169, column 4. See pages 202,204 of PBP. (Severity: 5)
Variable declared in conditional statement at line 1571, column 3. Declare variables outside of the condition. (Severity: 5)
Stricture disabled at line 1682, column 2. See page 429 of PBP. (Severity: 5)
Don't modify $_ in list functions at line 1815, column 11. See page 114 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 1891, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 1969, column 2. See page 199 of PBP. (Severity: 5)
Variable declared in conditional statement at line 1990, column 3. Declare variables outside of the condition. (Severity: 5)
Nested named subroutine at line 2135, column 2. Declaring a named sub inside another named sub does not prevent the inner sub from being global. (Severity: 5)
"return" statement with explicit "undef" at line 2140, column 3. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 2663, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 2671, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 2682, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 2753, column 2. See page 199 of PBP. (Severity: 5)
Nested named subroutine at line 2789, column 2. Declaring a named sub inside another named sub does not prevent the inner sub from being global. (Severity: 5)
"return" statement with explicit "undef" at line 3373, column 3. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 3707, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 3723, column 3. See page 199 of PBP. (Severity: 5)
Bareword file handle opened at line 3977, column 3. See pages 202,204 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4116, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4119, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4204, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4212, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4220, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4228, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4244, column 2. See page 199 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 4291, column 2. See page 199 of PBP. (Severity: 5)
Nested named subroutine at line 4463, column 2. Declaring a named sub inside another named sub does not prevent the inner sub from being global. (Severity: 5)
Subroutine prototypes used at line 4707, column 1. See page 194 of PBP. (Severity: 5)
Stricture disabled at line 4817, column 2. See page 429 of PBP. (Severity: 5)
Subroutine prototypes used at line 4831, column 1. See page 194 of PBP. (Severity: 5)
Nested named subroutine at line 5160, column 3. Declaring a named sub inside another named sub does not prevent the inner sub from being global. (Severity: 5)
Nested named subroutine at line 5271, column 2. Declaring a named sub inside another named sub does not prevent the inner sub from being global. (Severity: 5)
Use IO::Interactive::is_interactive() instead of -t at line 5477, column 8. See page 218 of PBP. (Severity: 5)
"return" statement with explicit "undef" at line 5823, column 2. See page 199 of PBP. (Severity: 5)
Glob written as <...> at line 5904, column 11. See page 167 of PBP. (Severity: 5)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: git svn perl issues
2011-03-23 15:52 git svn perl issues Stephen Hemminger
@ 2011-03-23 21:38 ` Lasse Makholm
2011-03-23 21:45 ` Stephen Hemminger
0 siblings, 1 reply; 4+ messages in thread
From: Lasse Makholm @ 2011-03-23 21:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: git
On 23 March 2011 16:52, Stephen Hemminger <shemminger@vyatta.com> wrote:
> 1. The following needs to be fixed:
>
> $ git svn clone
> Use of uninitialized value $_[0] in substitution (s///) at /usr/share/perl/5.10.1/File/Basename.pm line 341.
> fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line 403
While noisy and ugly, uninitialized warnings are usually pretty harmless...
> 2. The git-svn perl script does not follow Perl Best Practices.
> If you run the perlcritic script on it, all the following warnings/errors
> are generated:
Some of these are undoubtedly valid complaints, but the so called best
practices that the perl critic policies implement are, in my opinion,
not widely accepted as such by the perl community. At least not all of
them. I wouldn't go following them blindly - especially in working
production code...
/Lasse
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: git svn perl issues
2011-03-23 21:38 ` Lasse Makholm
@ 2011-03-23 21:45 ` Stephen Hemminger
2011-03-23 22:19 ` Lasse Makholm
0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2011-03-23 21:45 UTC (permalink / raw)
To: Lasse Makholm; +Cc: git
> On 23 March 2011 16:52, Stephen Hemminger <shemminger@vyatta.com>
> wrote:
> > 1. The following needs to be fixed:
> >
> > $ git svn clone
> > Use of uninitialized value $_[0] in substitution (s///) at
> > /usr/share/perl/5.10.1/File/Basename.pm line 341.
> > fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line
> > 403
>
> While noisy and ugly, uninitialized warnings are usually pretty
> harmless...
User should never see perl splat, it is sloppy.
> > 2. The git-svn perl script does not follow Perl Best Practices.
> > If you run the perlcritic script on it, all the following
> > warnings/errors
> > are generated:
>
> Some of these are undoubtedly valid complaints, but the so called best
> practices that the perl critic policies implement are, in my opinion,
> not widely accepted as such by the perl community. At least not all of
> them. I wouldn't go following them blindly - especially in working
> production code...
Some of them are crap, but like sparse warnings it is trivial to
fix them and make it clean so why not.
If you don't maintain code it just rots.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: git svn perl issues
2011-03-23 21:45 ` Stephen Hemminger
@ 2011-03-23 22:19 ` Lasse Makholm
0 siblings, 0 replies; 4+ messages in thread
From: Lasse Makholm @ 2011-03-23 22:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: git
On 23 March 2011 22:45, Stephen Hemminger <stephen.hemminger@vyatta.com> wrote:
>
>> On 23 March 2011 16:52, Stephen Hemminger <shemminger@vyatta.com>
>> wrote:
>> > 1. The following needs to be fixed:
>> >
>> > $ git svn clone
>> > Use of uninitialized value $_[0] in substitution (s///) at
>> > /usr/share/perl/5.10.1/File/Basename.pm line 341.
>> > fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line
>> > 403
>>
>> While noisy and ugly, uninitialized warnings are usually pretty
>> harmless...
>
> User should never see perl splat, it is sloppy.
Agreed.
>> > 2. The git-svn perl script does not follow Perl Best Practices.
>> > If you run the perlcritic script on it, all the following
>> > warnings/errors
>> > are generated:
>>
>> Some of these are undoubtedly valid complaints, but the so called best
>> practices that the perl critic policies implement are, in my opinion,
>> not widely accepted as such by the perl community. At least not all of
>> them. I wouldn't go following them blindly - especially in working
>> production code...
>
> Some of them are crap, but like sparse warnings it is trivial to
> fix them and make it clean so why not.
Well, personally I don't think they all add any value but that's a bit
beside the point here...
> If you don't maintain code it just rots.
True enough. That said, I haven't actually looked into the git-svn
code oh my god why do people write 6K line scripts... *sigh*
My first suggestion would be to split it... :-) It's already 75%
classes anyway...
[forgot to reply all, sorry for the spam Stephen...]
--
/Lasse
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-23 22:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-23 15:52 git svn perl issues Stephen Hemminger
2011-03-23 21:38 ` Lasse Makholm
2011-03-23 21:45 ` Stephen Hemminger
2011-03-23 22:19 ` Lasse Makholm
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 NNTP newsgroup(s).