public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg•org>
To: "Derick W. de M. Frias" <derick.william.moraes@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH v2 4/4] t4018: add tests for javascript export type function declarations
Date: Thu, 26 Jun 2025 08:21:48 +0200	[thread overview]
Message-ID: <df3f9190-79cd-49d6-934c-a67a2c0c9f0d@kdbg.org> (raw)
In-Reply-To: <20250623090538.154858-5-derick.william.moraes@gmail.com>

Many of the cases are not valid tests. In particular the word "ChangeMe"
must occur *only once* and *after* the line with "RIGHT" and there must
be at least one unchanged line before it. (I wonder how these tests
could have passed. Do we have a flaw in the test driver?)

> diff --git a/t/t4018/javascript-dotexpors-async-anonymous-function b/t/t4018/javascript-dotexpors-async-anonymous-function
> new file mode 100644
> index 0000000000..9f970a2343
> --- /dev/null
> +++ b/t/t4018/javascript-dotexpors-async-anonymous-function
> @@ -0,0 +1,3 @@
> +exports.RIGHT = async function(a, b) {
> +    return a + b; // ChangeMe
> +};

Here.

> diff --git a/t/t4018/javascript-dotexports-anonymous-function b/t/t4018/javascript-dotexports-anonymous-function
> new file mode 100644
> index 0000000000..2fa9775c95
> --- /dev/null
> +++ b/t/t4018/javascript-dotexports-anonymous-function
> @@ -0,0 +1,3 @@
> +exports.RIGHT = function(a, b) {
> +    return a + b; //ChangeMe
> +};

Here.

> diff --git a/t/t4018/javascript-dotexports-arrow-function-3 b/t/t4018/javascript-dotexports-arrow-function-3
> new file mode 100644
> index 0000000000..cc3f1ec017
> --- /dev/null
> +++ b/t/t4018/javascript-dotexports-arrow-function-3
> @@ -0,0 +1 @@
> +exports.RIGHT = a => a+1; //ChangeMe

Here.

And many more. You see the pattern.

> +++ b/t/t4018/javascript-module-dotexports-generator-function
> @@ -0,0 +1,5 @@
> +module.exports.RIGHT = function* ChangeMe() {
> +
> +    yield 1;
> +    yield 2;
> +}
> \ No newline at end of file

An incomplete last line, again. Please look at the patch text that you
are going to submit. Don't depend on reviewers' to notice such obvious
glitches.

I haven't found enough time for a complete review, yet. Please submit a
patch series that does not depend on an earlier round.

Splitting the test cases in separate patches is a good idea. I think you
have chosen a split that excercises different hunk header patterns. But
in this case, it would also be feasible and helpful to exclude the
pattern from the earlier patch and add the pattern and the corresponding
test cases in the same commit.

-- Hannes


      reply	other threads:[~2025-06-26  6:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04  9:35 [GSoC PATCH 0/1] userdiff: add javascript diff driver Derick W. de M. Frias
2025-06-04  9:35 ` [GSoC PATCH 1/1] " Derick W. de M. Frias
2025-06-04 21:19   ` D. Ben Knoble
2025-06-05  5:51   ` Johannes Sixt
2025-06-23  6:35     ` [PATCH v2 0/4] diff: create pattern for javascript language Derick W. de M. Frias
2025-06-23  6:35       ` [PATCH v2 1/4] userdiff: add javascript diff driver Derick W. de M. Frias
2025-06-23 18:09         ` Junio C Hamano
2025-06-23  6:35       ` [PATCH v2 2/4] t4034: add tests for javascript word literals Derick W. de M. Frias
2025-06-23  6:35       ` [PATCH v2 3/4] t4018: add tests for recognizing javascript function syntax Derick W. de M. Frias
2025-06-23  6:35       ` [PATCH v2 4/4] t4018: add tests for javascript export type function declarations Derick W. de M. Frias
2025-06-26  6:21         ` Johannes Sixt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=df3f9190-79cd-49d6-934c-a67a2c0c9f0d@kdbg.org \
    --to=j6t@kdbg$(echo .)org \
    --cc=derick.william.moraes@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox