Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 19, 2015, 21:53 UTC
Message-ID: <CAPig+cS3rZgTZRYsC2ofNAFHs9WsGO1AqW01dt734AZf6edBzQ@mail.gmail.com>
In-Reply-To: <CAPig+cR-87-dWXUKoKvphrdb_Y=7268G5uerAiPYDsj-UQiuRw@mail.gmail.com>

On Wed, Mar 18, 2015 at 3:26 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 29 quoted lines

> On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan <pyokagan@gmail.com> wrote:
>> t0302 now tests git-credential-store's support for the XDG user-specific
>> configuration file $XDG_CONFIG_HOME/git/credentials. Specifically:
>> ---
>>
>> The previous version can be found at [1].
>>
>> [1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265308
>>
>> * Merge related, but previously separate, tests together in order to
>>   make the test suite easier to understand.
>>
>> * Instead of setting/unsetting XDG_CONFIG_HOME in separate tests, set
>>   it, and unset it immediately before and after "helper_test store" is
>>   called in order to make it localized to only the command that it
>>   should affect.
>>
>> * Add test, previously missing, to check that only the home credentials
>>   file is written to if both the xdg and home files exist.
>>
>> * Correct mislabelling of "home-user"/"home-pass" to the proper
>>   "xdg-user"/"xdg-pass".
>>
>> * Use "rm -f" instead of "test_might_fail rm".
>
> This round looks much better. Thanks.
>
> Most of the comments below are just nit-picky, with one or two genuine
> (minor) issues.

I should add that the nit-picky items are not necessarily actionable. As the person doing the actual work, it's okay if you disagree and feel that they are not worth the effort of addressing.

(The genuine issues, on the other hand, ought to be addressed.)