From: Thomas Adam Date: Tue, 07 Jun 2011 10:16:30 GMT Subject: Re: [PATCHv2] Add a remote helper to interact with mediawiki, pull & clone handled Message-ID: In-Reply-To: <1307440754-3697-1-git-send-email-jeremie.nikaes@ensimag.imag.fr> Hi, On 7 June 2011 10:59, Jeremie Nikaes wrote: > +       @cmd = split(/ /,$entry); Hmm. What guarantees can you make about the scalar value of @cmd here once the split has happened? Do you not care, or do you have a hard-limit for split to use? > +                       print STDERR "Unknown capability. Aborting...\n"; warn()? > +       print STDOUT "refspec refs/heads/*:refs/mediawiki/$remotename/*\n"; > +       print STDOUT "import\n"; > +       print STDOUT "list\n"; > +       print STDOUT "option\n"; > +       print STDOUT "push\n"; > +       print STDOUT "\n"; These explicit calls to "STDOUT" here -- why? It's redundant typing and unsightly, and certainly not very perl-y. Is there some reason why you're using this style? > +       print STDOUT "? refs/heads/master\n"; > +       print STDOUT '@'."refs/heads/master HEAD\n"; > +       print STDOUT "\n"; Hmm. Why not just: print STDOUT '@'."refs/heads/master HEAD\n\n"; There's certainly a large number of perl-specific clean-ups I'd be inclined to do -- and if I get time later, I might show you how. But don't let this necessarily put off this patch for inclusion or anything like that. -- Thomas Adam