Re: [PATCH v3 1/4] git-credential-store: support multiple credential files

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

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Paul Tan <pyokagan@xxxxxxxxx> writes:
>
>>> I think you could even get away without passing default_fn here, and
>>> just use the rule "the first file in the list is the default". Unless
>>> you are anticipating ever passing something else, but I couldn't think
>>> of a case where that would be useful.
>>
>> Even though in this case the store_credential() function is not used
>> anywhere else, from my personal API design experience I think that
>> cementing the rule of "the first file in the list is the default" in
>> the behavior of the function is not a good thing. For example, in the
>> future, we may wish to keep the precedence ordering the same, but if
>> none of the credential files exist, we create the XDG file by default
>> instead.
>
> I am not sure if this is not a premature over-engineering

I would say so if having this default_fn made the code more complex, but
here the code is basically

+	if (default_fn)
+		store_credential_file(default_fn, c);

and

-		store_credential(file, &c);
+		store_credential(&fns, &c, fns.items[0].string);

Taking the first element in the list wouldn't change much.

I'm personally fine with both versions.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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]