On Mon, Feb 11, 2013 at 06:14:24PM +0100, Michal Nazarewicz wrote:
> > Should this return a hash reference? It seems like that is how we end up
> > using and passing it elsewhere (since we have to anyway when passing it
> > as a parameter).
>
> Admittedly I mostly just copied what git-remote-mediawiki did here and
> don't really have any preference either way, even though with this
> function returning a reference the call site would have to become:
>
> %$credential = %{ credential_read $reader };
Oh, right, because Git::credential takes the credential as an in-out
parameter rather than just returning it. Which is a bit unusual in perl,
but keeps the interface reasonably simple. The alternative would be:
$cred = Git::credential $cred, sub {
...
}
which is a little less nice.
> Another alternative would be for it to take a reference as an argument,
> possibly an optional one:
I think that is making things more ugly.
> I'd avoid modifying the hash while reading though since I think it's
> best if it's left intact in case of an error.
Agreed.
> And of course, if we want to get even more crazy, credential_write could
> accept either reference or a hash, like so:
>
> +sub credential_write {
> + my ($self, $writer, @rest) = _maybe_self(@_);
> + my $credential = @rest == 1 ? $rest[0] : { @rest };
> + my ($key, $value);
> + # ...
> +}
Ugh.
> Bottom line is, anything can be coded, but a question is whether it
> makes sense to do so. ;)
Yes, it is probably OK to leave it as-is, then. It is largely a matter
of taste, and I will defer to your judgement on that. :)
-Peff
--
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