Shreyansh Paliwal <shreyanshpaliwalcmsmn@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
