Re: [Patch] test-lib-functions.sh : change test_i18ngrep to test_grep

[Kousik Sanagavarapu <five231003@gmail.com>]

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> wrote:

> Subject: [Patch] test-lib-functions.sh : change test_i18ngrep to test_grep

For anyone reading the subject, I think reading

	change test_i18ngrep to test_grep

would be confusing, as from the looks of it, the patch does remove
test_i18ngrep() and replace it with test_grep (I mean the plan is to
remove test_i18ngrep only after we are sure that it doesn't exist in the
code anywhere, anymore) but only making a change in the wording of an
error message within test_grep().

Also I think we can drop the SP after "related topic" part of the patch
and the colon (but have the SP after the colon), that is

	"test-lib-functions.sh: ..."

Also, nit, but I think we should have [PATCH] instead of [Patch]. I'm not
really sure if Junio's setup treats [PATCH] and [Patch] to be same :)

> Recently the test_i18ngrep was deprecated from the source code and
> test_grep was implemented but in the test-lib-functions.sh file , in
> the test_grep() function definition,

This recent deprecation was made in the commit,
2e87fca189 (test framework: further deprecate test_i18ngrep, 2023-10-31)
and it makes sense to include it in the commit message as the following
change is essentially something that the previous commit seems to have
forgotten to do.

> it is written BUG "too few parameters to test_i18ngrep".

I think it is not necessary to mention what is the current code
in _this case_ as it can be read in the change itself :)

> So the following patch solves the minor problem.

What exactly is the problem? I think it should be mentioned in the commit
message that the wording of the error message causes confusion ;) as when
test_grep() is used in a test and this test fails. That the change is - it
would be clear to see

	"too few parameters to test_grep"

instead of

	"too few parameters to test_i18ngrep"

> Signed-off-by: Shreyansh Paliwal <Shreyanshpaliwalcmsmn@gmail.com>
> ---
>  t/test-lib-functions.sh | 2 +-
>  1 file changed, 1 insertions(+), 1 deletions(-)
> 
>  t/test-lib-functions.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)diff --git
> a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 9c3cf12b26..8737c95e0c 100644
> --- a/t/test-lib-functions.sh
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1277,7 +1277,7 @@ test_grep () {
>         if test $# -lt 2 ||
>            { test "x!" = "x$1" && test $# -lt 3 ; }
>         then
> -               BUG "too few parameters to test_i18ngrep"
> +               BUG "too few parameters to test_grep"
>         fi
> 
>         if test "x!" = "x$1"
> --
> 2.43

The diff format doesn't seem proper (some repeated lines and no newlines
at the required places).

If you have no go-to tool to send patches through email then git-send-email
is a really good tool to do it. It handles most of the work for you.
"MyFirstContribution" has a guide to do so

	https://git-send-email.io/ (also has setup with GMail)
	https://git-scm.com/docs/MyFirstContribution#howto-git-send-email

Another good resource which is not linked often is

	https://flusp.ime.usp.br/git/sending-patches-by-email-with-git/

by Matheus Tavares, also a Git Contributor. It also has other useful links
which are worth a read.

Thanks