bug#70939: [PATCH] Add commands to run unit tests in go-ts-mode

[Randy Taylor]

On Saturday, July 6th, 2024 at 15:44, Ankit Gadiya <ankit@argp.in> wrote:
> 
> Apologies for the delay again.
> 
> > > > For the commit message, I'm not sure we need that paragraph especially 
> > > > when it's already described in the news. Eli what do you think?
> 
> 
> Thanks Randy and Eli, I've updated the commit message to be shorter now.
> 
> > > > +*** New unit test commands.
> > > > +Two new commands are now available to run unit tests.
> > > > Three?
> > 
> > This still needs to be updated.
> 
> 
> Thanks, I fixed it.
> 
> > A few more comments:
> > +(defun go-ts-mode--get-test-regexp-at-point ()
> > + "Return a regular expression for tests at point.
> > ^ the
> > 
> > Could go-ts-mode--get-test-regexp-at-point and go-ts-mode-test-file use 
> > if-let?
> 
> 
> I wasn't familiar with if-let, thanks for this. Incorporated it in
> both the functions.
> 
> > Also, the indentation looks off in go-ts-mode-test-function-at-point (2 
> > extra spaces methinks).
> 
> 
> Fixed this, thanks.
> 
> > > > I'm also wondering if we should include "current" in the 
> > > > go-ts-mode-test-file and go-ts-mode-test-package function names. Maybe 
> > > > someone would expect that they would get prompted to select a file or 
> > > > package to test? Maybe I'm overthinking that :). Eli what do you think?
> > > 
> > > I'll wait for Eli to reply before incorporating the changes :).
> > 
> > And he chimed in - let's go with his suggestions.
> 
> 
> Updated both the function names.
> go-ts-mode-test-this-file
> go-ts-mode-test-this-package
> 
> > > Additionally, I also noticed that the emacs-30 branch is cut. I wanted
> > > to check if I
> > > need to rebase my patch onto master or emacs-30 branch?
> > 
> > I would guess master, but let's see what Eli says.
> 
> 
> I rebased my patch on the master now. Requesting a re-review.
> 
> Thanks
> 
> --
> Ankit

I only have a few comments about the commit message:

Three new commands are added in the go-ts-mode to run unit tests.
I would just drop this line altogether, personally.

(go-ts-mode-map): New map variable.
This should probably read something like Add new bindings.

(go-ts-mode-test-file): New function.
(go-ts-mode-test-package): New function.
These two need to be updated (...-test-this-...).

Everything else looks good to me. Thanks for working on this, Ankit.

Eli, if you have no further comments please install when you get a chance. 
Thanks in advance.