GNU bug report logs - #75789
[PATCH Debbugs] Factor cache accesses into dedicated functions

Previous Next

Full log

View this message in rfc822 format

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Cc: 75789 <at> debbugs.gnu.org
Subject: bug#75789: [PATCH Debbugs] Factor cache accesses into dedicated functions
Date: Sun, 26 Jan 2025 11:45:29 +0100

Morgan Smith <Morgan.J.Smith <at> outlook.com> writes:

> Hello!

Hi Morgan,

> Just a little bit of code cleanup.  I hope this is helpful.
>
> You'll notice I removed a bit in `debbugs-newest-bugs' that was supposed to
> temporarily generate a null value.  I'm not entirely certain but I don't
> believe that actually does anything right?  Immediately after that we make a
> synchronous soap call anyways and then return that value.

I don't know. See the comment you've removed as well:

--8<---------------cut here---------------start------------->8---
> -	  ;; Due to `debbugs-gnu-completion-table', this function
> -	  ;; could be called in rapid sequence.  We cache temporarily
> -	  ;; the value nil, therefore.
--8<---------------cut here---------------end--------------->8---

Have you checked that this doesn't happen without the temporary setting?

Some few comments:

> index ce6489ca39..d0aa9e87b4 100644
> --- a/test/debbugs-tests.el
> +++ b/test/debbugs-tests.el
> @@ -28,6 +28,10 @@
>
>  (require 'debbugs)
>
> +;; TODO: This shouldn't be necessary but I get the error
> +;; "(void-variable debbugs-gnu-use-threads)" without this
> +(require 'debbugs-gnu)

Oops. This is an existing bug. I will try to fix it later.

> +(add-function
> + :around (symbol-function #'debbugs-get-cache)
> + #'debbugs-test--override-float-time)
> +
> +(add-function
> + :around (symbol-function #'debbugs-put-cache)
> + #'debbugs-test--override-float-time)

This is wrong as-it-is. The extended definitions of debbugs-*-cache will
persist after running the tests. A strict rule is, that all changes must
be reverted after running the tests.

I don't believe you need this. Keep the (cl-letf (((symbol-function
#'float-time) ...))) clause in debbugs-test-get-status, and do the same
in debbugs-test-newest-bug-cached.

Otherwise, it looks good to me. Please push to the debbugs repo; I'll
play with it for a while before making a new release.

Thanks, and best regards, Michael.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.