From: Eli Zaretskii <eliz@gnu.org>
To: Javier Olaechea <pirata@gmail.com>
Cc: 70145@debbugs.gnu.org, jp@neverwas.me
Subject: bug#70145: [PATCH] Add sqlite-execute-batch command
Date: Sat, 13 Apr 2024 11:03:53 +0300 [thread overview]
Message-ID: <86a5lxr8nq.fsf@gnu.org> (raw)
In-Reply-To: <CAFVS=zAVxJG2mbJ189zdgvh6kxgQdBini3tkZwJASXUKBocd8g@mail.gmail.com> (message from Javier Olaechea on Tue, 2 Apr 2024 21:42:06 -0500)
> From: Javier Olaechea <pirata@gmail.com>
> Date: Tue, 2 Apr 2024 21:42:06 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, 70145@debbugs.gnu.org
>
> Thanks for working on this. I think Emacs applications will find it
> handy for things like initializing a new database. Just a couple things
> I spotted in passing. (I don't really know anything, though, so feel
> free to ignore.)
>
> Feedback is always welcome. It's not like I know anything either ^_^'.
>
> I think there's also a test/src/sqlite-tests.el file, which seems to
> contain tests for those primitives defined in src/sqlite.c. Perhaps this
> test should go there instead?
>
> Thanks for pointing that out, I hadn't seen that file. It does seem like a better home for the test.
>
> > diff --git a/test/lisp/sqlite-tests.el b/test/lisp/sqlite-tests.el
> > index d4892a27efc..7053026eb82 100644
> > --- a/test/lisp/sqlite-tests.el
> > +++ b/test/lisp/sqlite-tests.el
> > @@ -48,4 +48,29 @@ with-sqlite-transaction/rollback
> > ;; First insertion (a=1) rolled back.
> > (should-not (sqlite-select db "select * from test"))))
> >
> > +(ert-deftest with-sqlite-execute-batch-test ()
>
> Maybe I'm mistaken, but I don't think you need the "with-" part (or the
> "-test" part), no?
>
> I did find it contrary to the foo-test- prefix I've seen in the elisp code in the wild, but on matters of naming
> conventions I try to follow the motto: "When in Rome, do as Romans do". So I named the test according to the
> other tests in the file. Now that I placed it under test/src/sqlite-tests.el I've changed the name to align with the
> names of the test in that file.
>
> I've attached a new patch incorporating your feedback
Thanks. This should have a NEWS entry announcing the new command.
Also, to accept this contribution, we'd need a copyright assignment
from you. Would you like to start the paperwork for that at this
time? If yes, I will send you the form to fill and the instructions
to go with it.
next prev parent reply other threads:[~2024-04-13 8:03 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-02 15:03 bug#70145: [PATCH] Add sqlite-execute-batch command Javier Olaechea
2024-04-02 16:21 ` Eli Zaretskii
2024-04-02 18:52 ` Javier Olaechea
2024-04-02 18:56 ` Eli Zaretskii
2024-04-02 19:10 ` Javier Olaechea
2024-04-03 1:56 ` J.P.
2024-04-03 2:42 ` Javier Olaechea
2024-04-13 8:03 ` Eli Zaretskii [this message]
2024-04-14 3:32 ` Javier Olaechea
2024-04-14 5:52 ` Eli Zaretskii
2024-06-06 8:00 ` Javier Olaechea
2024-06-06 10:08 ` Eli Zaretskii
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
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86a5lxr8nq.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=70145@debbugs.gnu.org \
--cc=jp@neverwas.me \
--cc=pirata@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).