{"thread":{"id":"26841","subject":"git svn perl issues","startedAt":"2011-03-23T15:52:37Z","lastAt":"2011-03-23T22:19:46Z","messageCount":4,"participants":["Stephen Hemminger","Lasse Makholm"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"164156","messageId":"20110323085237.1a52ab7e@nehalam","threadId":"26841","inReplyTo":null,"subject":"git svn perl issues","fromName":"Stephen Hemminger","fromEmail":"shemminger@vyatta.com","sentAt":"2011-03-23T15:52:37Z","receivedAt":"2011-03-23T15:52:37Z","isPatch":false,"sender":{"key":"shemminger@vyatta.com","avatar":null},"body":"1. The following needs to be fixed:\n\n$ git svn clone \nUse of uninitialized value $_[0] in substitution (s///) at /usr/share/perl/5.10.1/File/Basename.pm line 341.\nfileparse(): need a valid pathname at /usr/lib/git-core/git-svn line 403\n\n\n2. The git-svn perl script does not follow Perl Best Practices.\nIf you run the perlcritic script on it, all the following warnings/errors\nare generated:\n\n$ perlcritic /usr/lib/git-core/git-svn \nCode before strictures are enabled at line 2, column 10.  See page 429 of PBP.  (Severity: 5)\nVariable declared in conditional statement at line 18, column 1.  Declare variables outside of the condition.  (Severity: 5)\nSubroutine prototypes used at line 39, column 1.  See page 194 of PBP.  (Severity: 5)\nStricture disabled at line 65, column 2.  See page 429 of PBP.  (Severity: 5)\nVariable declared in conditional statement at line 287, column 1.  Declare variables outside of the condition.  (Severity: 5)\nVariable declared in conditional statement at line 533, column 2.  Declare variables outside of the condition.  (Severity: 5)\nBareword file handle opened at line 851, column 3.  See pages 202,204 of PBP.  (Severity: 5)\nBareword file handle opened at line 1169, column 4.  See pages 202,204 of PBP.  (Severity: 5)\nVariable declared in conditional statement at line 1571, column 3.  Declare variables outside of the condition.  (Severity: 5)\nStricture disabled at line 1682, column 2.  See page 429 of PBP.  (Severity: 5)\nDon't modify $_ in list functions at line 1815, column 11.  See page 114 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 1891, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 1969, column 2.  See page 199 of PBP.  (Severity: 5)\nVariable declared in conditional statement at line 1990, column 3.  Declare variables outside of the condition.  (Severity: 5)\nNested 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)\n\"return\" statement with explicit \"undef\" at line 2140, column 3.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 2663, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 2671, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 2682, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 2753, column 2.  See page 199 of PBP.  (Severity: 5)\nNested 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)\n\"return\" statement with explicit \"undef\" at line 3373, column 3.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 3707, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 3723, column 3.  See page 199 of PBP.  (Severity: 5)\nBareword file handle opened at line 3977, column 3.  See pages 202,204 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4116, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4119, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4204, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4212, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4220, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4228, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4244, column 2.  See page 199 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 4291, column 2.  See page 199 of PBP.  (Severity: 5)\nNested 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)\nSubroutine prototypes used at line 4707, column 1.  See page 194 of PBP.  (Severity: 5)\nStricture disabled at line 4817, column 2.  See page 429 of PBP.  (Severity: 5)\nSubroutine prototypes used at line 4831, column 1.  See page 194 of PBP.  (Severity: 5)\nNested 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)\nNested 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)\nUse IO::Interactive::is_interactive() instead of -t at line 5477, column 8.  See page 218 of PBP.  (Severity: 5)\n\"return\" statement with explicit \"undef\" at line 5823, column 2.  See page 199 of PBP.  (Severity: 5)\nGlob written as <...> at line 5904, column 11.  See page 167 of PBP.  (Severity: 5)\n"},{"id":"164185","messageId":"AANLkTikowuFsXFwLL14oS0zzHh3RiHOrGTVHXgyy8dLw@mail.gmail.com","threadId":"26841","inReplyTo":"20110323085237.1a52ab7e@nehalam","subject":"Re: git svn perl issues","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2011-03-23T21:38:35Z","receivedAt":"2011-03-23T21:38:35Z","isPatch":false,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"On 23 March 2011 16:52, Stephen Hemminger <shemminger@vyatta.com> wrote:\n> 1. The following needs to be fixed:\n>\n> $ git svn clone\n> Use of uninitialized value $_[0] in substitution (s///) at /usr/share/perl/5.10.1/File/Basename.pm line 341.\n> fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line 403\n\nWhile noisy and ugly, uninitialized warnings are usually pretty harmless...\n\n> 2. The git-svn perl script does not follow Perl Best Practices.\n> If you run the perlcritic script on it, all the following warnings/errors\n> are generated:\n\nSome of these are undoubtedly valid complaints, but the so called best\npractices that the perl critic policies implement are, in my opinion,\nnot widely accepted as such by the perl community. At least not all of\nthem. I wouldn't go following them blindly - especially in working\nproduction code...\n\n/Lasse\n"},{"id":"164187","messageId":"521251622.25680.1300916735091.JavaMail.root@tahiti.vyatta.com","threadId":"26841","inReplyTo":"AANLkTikowuFsXFwLL14oS0zzHh3RiHOrGTVHXgyy8dLw@mail.gmail.com","subject":"Re: git svn perl issues","fromName":"Stephen Hemminger","fromEmail":"stephen.hemminger@vyatta.com","sentAt":"2011-03-23T21:45:35Z","receivedAt":"2011-03-23T21:45:35Z","isPatch":false,"sender":{"key":"stephen.hemminger@vyatta.com","avatar":null},"body":"\n> On 23 March 2011 16:52, Stephen Hemminger <shemminger@vyatta.com>\n> wrote:\n> > 1. The following needs to be fixed:\n> >\n> > $ git svn clone\n> > Use of uninitialized value $_[0] in substitution (s///) at\n> > /usr/share/perl/5.10.1/File/Basename.pm line 341.\n> > fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line\n> > 403\n> \n> While noisy and ugly, uninitialized warnings are usually pretty\n> harmless...\n\nUser should never see perl splat, it is sloppy.\n\n> > 2. The git-svn perl script does not follow Perl Best Practices.\n> > If you run the perlcritic script on it, all the following\n> > warnings/errors\n> > are generated:\n> \n> Some of these are undoubtedly valid complaints, but the so called best\n> practices that the perl critic policies implement are, in my opinion,\n> not widely accepted as such by the perl community. At least not all of\n> them. I wouldn't go following them blindly - especially in working\n> production code...\n\nSome of them are crap, but like sparse warnings it is trivial to\nfix them and make it clean so why not.\n\nIf you don't maintain code it just rots.\n"},{"id":"164188","messageId":"AANLkTim3yK2=MjO1NbpQ2pu4tV7=hwR-Z9UbixdfAkm=@mail.gmail.com","threadId":"26841","inReplyTo":"521251622.25680.1300916735091.JavaMail.root@tahiti.vyatta.com","subject":"Re: git svn perl issues","fromName":"Lasse Makholm","fromEmail":"lasse.makholm@gmail.com","sentAt":"2011-03-23T22:19:46Z","receivedAt":"2011-03-23T22:19:46Z","isPatch":false,"sender":{"key":"lasse.makholm@gmail.com","avatar":"https://gravatar.com/avatar/5bc3a34e4fa8ba26b38c333f370b15c034351e18e09b056a2608f3904ae1a4c1?d=mp&s=160"},"body":"On 23 March 2011 22:45, Stephen Hemminger <stephen.hemminger@vyatta.com> wrote:\n>\n>> On 23 March 2011 16:52, Stephen Hemminger <shemminger@vyatta.com>\n>> wrote:\n>> > 1. The following needs to be fixed:\n>> >\n>> > $ git svn clone\n>> > Use of uninitialized value $_[0] in substitution (s///) at\n>> > /usr/share/perl/5.10.1/File/Basename.pm line 341.\n>> > fileparse(): need a valid pathname at /usr/lib/git-core/git-svn line\n>> > 403\n>>\n>> While noisy and ugly, uninitialized warnings are usually pretty\n>> harmless...\n>\n> User should never see perl splat, it is sloppy.\n\nAgreed.\n\n>> > 2. The git-svn perl script does not follow Perl Best Practices.\n>> > If you run the perlcritic script on it, all the following\n>> > warnings/errors\n>> > are generated:\n>>\n>> Some of these are undoubtedly valid complaints, but the so called best\n>> practices that the perl critic policies implement are, in my opinion,\n>> not widely accepted as such by the perl community. At least not all of\n>> them. I wouldn't go following them blindly - especially in working\n>> production code...\n>\n> Some of them are crap, but like sparse warnings it is trivial to\n> fix them and make it clean so why not.\n\nWell, personally I don't think they all add any value but that's a bit\nbeside the point here...\n\n> If you don't maintain code it just rots.\n\nTrue enough. That said, I haven't actually looked into the git-svn\ncode oh my god why do people write 6K line scripts... *sigh*\n\nMy first suggestion would be to split it... :-) It's already 75%\nclasses anyway...\n\n[forgot to reply all, sorry for the spam Stephen...]\n-- \n/Lasse\n"}]}