emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: lra@phdk.org
Cc: Karthik Chikmagalur <karthikchikmagalur@gmail.com>,
	Emacs Orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] Fixing a regression in org-persists ability to handle non-list container arguments.
Date: Sun, 06 Oct 2024 12:35:37 +0000	[thread overview]
Message-ID: <87bjzxtnmu.fsf@localhost> (raw)
In-Reply-To: <O8IU9xk--B-9@phdk.org>

lra@phdk.org writes:

>> P.S. We really need tests for org-persist.
>
> Yeah I did note a couple of pretty straight-forward opportunities for
> clean-up wrt. redundancies and readability, but without
> regression-tests and lacking confidence that I understood everything I
> thought it was best to keep the patch as simple as possible. I
> would've felt pretty stupid if I accidentally introduced a new
> regression in my regression-fix haha. I'll include some of the stuff
> from my notes below in case it's of interest to anybody working on
> org-persist.
> ...

It is hard to judge many of the comments without seeing the right
context. Ideally, the proposed ideas could be done in a form of patches
;)

That said, I would be similarly hesitant to merge large changes in
org-persist without adding at least some basic tests.

Would you be interested to create testing/lisp/org-persist.el file and
add one or two basic tests that replicate what the preview branch does?
That way, we can at least make sure that things are not broken as we
change the actual library.

Then, we can start looking into changing org-persist itself. (Also,
having _a_ test file will greatly reduce friction to add more tests)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>


      reply	other threads:[~2024-10-06 12:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-18  0:24 [PATCH] Fixing a regression in org-persists ability to handle non-list container arguments lra
     [not found] ` <O71W8io--B-9@phdk.org-O71WWax----9>
2024-09-18  0:29   ` lra
2024-09-22  9:11     ` Ihor Radchenko
2024-09-22 23:04       ` Karthik Chikmagalur
2024-10-02 17:34         ` Ihor Radchenko
2024-10-03 18:00           ` lra
2024-10-06 12:35             ` Ihor Radchenko [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

  List information: https://www.orgmode.org/

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

  git send-email \
    --in-reply-to=87bjzxtnmu.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=emacs-orgmode@gnu.org \
    --cc=karthikchikmagalur@gmail.com \
    --cc=lra@phdk.org \
    /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/org-mode.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).