Re: [PATCHv2] Add a remote helper to interact with mediawiki, pull & clone handled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 7 June 2011 10:59, Jeremie Nikaes <jeremie.nikaes@xxxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]