Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> mk_test() creates a repository with the constant name "testrepo", and
>> this may be limiting for tests that need to create more than one
>> repository for testing. To fix this, create a new mk_test_with_name()
>> which accepts the repository name as $1. Reimplement mk_test() as a
>> special case of this function, making sure that no tests need to be
>> rewritten.
>
> Why not give mk_test an optional parameter?
>
> repo_name=${1:-testrepo}
>
> Oh, it is because mk_test already takes arguments naming refs to push.
> I suppose the change description could make this clearer.
Isn't it obvious?
> Why not use mk_test and then rename, like so?
>
> mk_test ...refs... &&
> mv testrepo testrepo-a &&
>
> mk_test ...refs... &&
> mv testrepo testrepo-b &&
> ...
No. This is ugly. mk_test() should not hardcode "testrepo".
> I dunno. The helper functions at the top of this test are already
> intimidating, so I guess I am looking for a way to avoid making that
> problem worse. One way would be to add an opening comment before
> the function definition explaining how it is meant to be used. See
> t/test-lib-functions.sh for examples, such as test_cmp.
My patch does not make the situation worse in any way: it just adds
one line that passes $1 as a parameter to existing code. Yes, the
functions and tests can be improved greatly, but I refrained from
doing so because of your series. We can save it for later.
--
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
