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

[Jeff King <peff@peff.net>]

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