From: Lasse Makholm <lasse.makholm@gmail.com>
To: Stephen Hemminger <stephen.hemminger@vyatta.com>
Cc: git@vger.kernel.org
Subject: Re: git svn perl issues
Date: Wed, 23 Mar 2011 23:19:46 +0100 [thread overview]
Message-ID: <AANLkTim3yK2=MjO1NbpQ2pu4tV7=hwR-Z9UbixdfAkm=@mail.gmail.com> (raw)
In-Reply-To: <521251622.25680.1300916735091.JavaMail.root@tahiti.vyatta.com>
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
prev parent reply other threads:[~2011-03-23 22:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='AANLkTim3yK2=MjO1NbpQ2pu4tV7=hwR-Z9UbixdfAkm=@mail.gmail.com' \
--to=lasse.makholm@gmail.com \
--cc=git@vger.kernel.org \
--cc=stephen.hemminger@vyatta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).