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.


> 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);
> +     # ...
> +}


> 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. :)

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to