Re: [PATCH] tests: Introduce test_seq
- From
Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
- Date
- Aug 6, 2012, 17:52 UTC
- Message-ID
- <20120806195256.43ec44de@gmail.com>
- In-Reply-To
- <501D4FF0.4060109@kdbg.org>
Johannes Sixt <j6t@kdbg.org> wrote:
Show 18 quoted lines
> Am 04.08.2012 00:09, schrieb Michał Kiedrowicz: > > Junio C Hamano <gitster@pobox.com> wrote: > >> I do not have strong > >> opinion on calling this test_seq when it acts differently from seq; > >> it is not confusing enough to make me push something longer that is > >> different from "seq", e.g. test_sequence. > > > > I prefer "test_seq" because it reminds seq which helps learning how to > > use it. If some other seq feature is ever needed (e.g. increment value, > > decrementing), it may be added at any time (but I don't think so, there > > are only few usages after years of test suite existence). > > And the reason for this is that we always told people "don't use seq" > and they submitted an updated patch. What would we have to do now? We > have to tell them "don't use seq, use test_seq". Therefore, the patch > does not accomplish anything useful, IMO. > > The function should really just be named 'seq'.
My reasoning was that there is already test_cmp, so let's make test_seq, but I agree with you that it doesn't solve the issue completely. So my 2 cents is that it would be best to stay with not allowing seq in the test suite.
Show 18 quoted lines
>
> Or how about this strategy:
>
> seq () {
> unset -f seq
> if ! seq 1 2 >/dev/null 2>&1
> then
> # don't have a working seq; provide it as a function
> seq () {
> insert your definition here
> }
> fi
> seq "$@"
> }
>
> but it is not my favorite.
>
> -- Hannes