One question about test patches. Are you ok with test_expect_failure
tests that document the expected failure of a feature yet to be
developed, followed by the feature, followed by the patch that makes
the tests into test_expect_success tests, or would you prefer to see
the pre- and post- test patches rolled into a single test that is
delivered after the feature patch?
On Thu, Aug 5, 2010 at 3:23 PM, Jon Seymour <jon.seymour@xxxxxxxxx> wrote:
> Junio,
>
> Thanks for the feedback. I'll rework along the lines you suggest. If
> it makes sense to make the other stash commands tolerant of non-stash
> entry references I'll add tests, support and documentation for that.
>
> jon.
>
> On Thu, August 2010 at 9:51 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Jon Seymour <jon.seymour@xxxxxxxxx> writes:
>>
>>> This patch allows git stash branch to work with stash-like commits created by git stash create.
>>>
>>> Two changes were required:
>>>
>>> * relax the pre-condition so that a stash stack is required if and only if a stash argument is not specified
>>> * don't attempt to drop a stash argument that doesn't look like a stash reference.
>>>
>>>
>>> Signed-off-by: Jon Seymour <jon.seymour@xxxxxxxxx>
>>
>> Please wrap very long lines.
>>
>>> diff --git a/git-stash.sh b/git-stash.sh
>>> index 1d95447..432ddae 100755
>>> --- a/git-stash.sh
>>> +++ b/git-stash.sh
>>> @@ -225,6 +225,12 @@ show_stash () {
>>> git diff $flags $b_commit $w_commit
>>> }
>>>
>>> +if_stash_ref() {
>>> + ref="$1"
>>> + shift
>>> + test "${ref#stash}" = "${ref}" -a "${ref#$ref_stash}" = "${ref}" || "$@"
>>> +}
>>
>> The interface to this function looks a rather bad taste to me; wouldn't it
>> look more natural if the callers can say:
>>
>> if stash_ref $it
>> then
>> do this
>> fi
>>
>> Your criteria used here is that the given parameter does not begin with
>> "stash" nor "refs/stash". If it begins with either of these two strings,
>> the "test" fails and "$@" is run. Wouldn't this produce a false hit if
>> you kept a handcrafted stash-looking commit with a tag "stash-42" or
>> something?
>>
>> It may make more sense to give "stash drop" an option to be silent if
>> the given parameter is not on the list to begin with, perhaps?
>>
>>
>
--
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