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

On Sat, Mar 14, 2015 at 01:33:28PM -0400, Jeff King wrote:

On Sat, Mar 14, 2015 at 04:15:53PM +0800, Paul Tan wrote:

quoted

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. It's a balance of flexibility, but in this case I think
putting the default filename in a separate argument is a good thing.

Yeah, I see your line of reasoning. I think this is probably a case of
YAGNI, but it is really a matter of personal preference. It's not a big
deal either way.

By the way, I hope this (and the other comment) do not come off as "you
are wrong, but I do not feel like arguing with you". I really do think
these are a matter of taste, and while we often express issues of taste
in reviews, it is ultimately up to the patch submitter (who is, after
all, doing most of the work) to have the final say on minor issues like
this.  Sometimes the response to taste issue is "oh, I didn't think to
do that, thanks for the suggestion" and sometimes it is "nah, I like it
better my way". And both are OK.

Of course there are also taste issues where we insist (e.g., consistent
whitespace), but I do not think this is one of them.  :)

Maybe that was all obvious, but since you are new to the list, I wanted
to make sure I was clear.

-Peff