On 3/24/2022 4:48 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>> All existing tests continue to pass with this change. There was one
>> instance from t7700-repack.sh that was taking advantage of this
>> flexibility, but it was removed in the previous change.
>
> Of course all existing tests continue to pass, as we no longer have
> any user of test_subcommand_inexact after the previous step ;-).
Yeah, I definitely should have checked to see if there were other
uses of this. I thought there was, but I was mistaken.
> Among
>
> (1) doing nothing,
> (2) removing, and
> (3) clarifying the implementation,
>
> my preference would be 2 > 1 > 3. If we add
I agree that (2) is the best option here.
> (4) clarify the implementation and document what kind of inexactness we
> tolerate with an updated comment"
>
> to the mix, that would come before all 3 others, though.
Is there value in fixing the implementation and adding this comment
if we are to just delete the helper? I suppose that we could prevent
a future contribution from reintroducing the broken implementation.
> Perhaps squash something like this in?
>
> t/test-lib-functions.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git i/t/test-lib-functions.sh w/t/test-lib-functions.sh
> index 0f439c99d6..6f6afae847 100644
> --- i/t/test-lib-functions.sh
> +++ w/t/test-lib-functions.sh
> @@ -1789,8 +1789,8 @@ test_subcommand () {
> }
>
> # Check that the given command was invoked as part of the
> -# trace2-format trace on stdin, but without an exact set of
> -# arguments.
> +# trace2-format trace on stdin, but only require that the
> +# initial arguments are given as specified.
This is an accurate description of what the fixed implementation
does.
My current feeling is that we should just delete this and refer
to that deletion if anyone considers needing something like it.
Thanks,
-Stolee
