On Sat, Mar 21, 2015 at 1:46 AM, Paul Tan <pyoka...@gmail.com> wrote:
> On Thu, Mar 19, 2015 at 3:26 AM, Eric Sunshine <sunsh...@sunshineco.com>
> wrote:
>> On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan <pyoka...@gmail.com> wrote:
>>> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
>>> index f61b40c..5b0a666 100755
>>> --- a/t/t0302-credential-store.sh
>>> +++ b/t/t0302-credential-store.sh
>>> @@ -6,4 +6,115 @@ test_description='credential-store tests'
>>> +test_expect_success 'when xdg credentials file does not exist, only write
>>> to ~/.git-credentials; do not create xdg file' '
>>
>> These test descriptions are quite long, often mirroring in prose what
>> the test body already says more concisely. I generally try to keep
>> descriptions short while still being descriptive enough to give a
>> general idea about what is being tested. I've rewritten a few test
>> descriptions (below) to be very short, in fact probably too terse; but
>> perhaps better descriptions would lie somewhere between the two
>> extremes. First example, for this test:
>>
>> "!xdg: >.git-credentials !xdg"
>>
> I will make the test descriptions shorter. However, I don't think the
> test descriptions need to be so terse such that a separate key is
> required. e.g. I will shorten the above to "when xdg file does not
> exist, it's not created.", or even terser: "when xdg file not exists,
> it's not created.". I don't think symbols should be used, as many
> other test descriptions do not use them.
Your updated test descriptions all sound fine.
>>> +XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME && helper_test store
>>> +unset XDG_CONFIG_HOME
>>
>> It's hard to spot the "helper_test store" at the end of line. I'd
>> place it on a line by itself so that it is easy to see that it is
>> wrapped by the setting and unsetting of the environment variable.
>
> Thanks, will fix. Although now it looks weird that the "export" is the
> only one with a continuation on a single line, so I split all of them
> so that they each have their own line.
An &&-chain outside of a test is not meaningful in this case, so I
meant either this:
XDG_CONFIG_HOME="$HOME/xdg"
export XDG_CONFIG_HOME
helper_test store
unset XDG_CONFIG_HOME
or, slightly more compact (using && just to combine the assignment and
export on one line):
XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME
helper_test store
unset XDG_CONFIG_HOME
--
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
