Re: [PATCHv2] Add a remote helper to interact with mediawiki, pull & clone handled
- From
Thomas Adam <thomas@xteddy.org>
- Date
- Jun 7, 2011, 10:16 UTC
- Message-ID
- <BANLkTik--BAqZxCYQ-ApUn2yKmcr_-hubg@mail.gmail.com>
- In-Reply-To
- <1307440754-3697-1-git-send-email-jeremie.nikaes@ensimag.imag.fr>
Hi,
On 7 June 2011 10:59, Jeremie Nikaes <jeremie.nikaes@ensimag.imag.fr> 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()?
Show 6 quoted lines
> + 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