Subject: Re: [bug#78063] [PATCH electronics-team] gnu: Add prjtrellis.
From: Maxim Cournoyer <[email-masked]>
To: Cayetano Santos <[email-masked]>
Cc: Ekaitz Zarraga <[email-masked]>, [email-masked]
Date: Thu, 08 May 2025 14:49:09 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi,

Cayetano Santos <[email-masked]> writes:

>>ven. 02 mai 2025 at 16:19, Maxim Cournoyer <[email-masked]> wrote:
>
> Hi Maxim,
>
>>> +      #:tests? #f ; tests are to be run from nextpnr-ecp5
>>
>> nitpick: leave at least two spaces between the code and the ';' margin
>> comment, as ensured by Emacs with 'M-;'.  Also, I'm not sure I get what
>> it means: the test are run from another package?  Could you explain a
>> bit more in a standalone comment above the argument?
>
> The idea is the following, see #77114. We spin-off a common nextpnr
> package, from which nextpnr-ice40 inherits, using icestorm as a backend.
>
> We do the same with nextpnr-ecp5, which inherits from nextpnr, and uses
> prjtrellis as a backend. Others will follow, see [0].
>
> This makes that example tests in prjtrellis, are to be tested under
> nextpnr-ecp5. Prjtrellis, by itself, cannot do much. Hope it is clear.
>
>> Otherwise it looks good to me.  Could you please submit a v2?
>
> Thanks for your review, I just sent v2, hopefully, including all your
> improvements.

Looks good!  I've made the standalone comments punctuation, and turned
the table into an itemized list (a table has two columns):

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/electronics.scm
@@ -432,8 +432,8 @@ (define-public prjtrellis
        (uri (git-reference
              (url "https://github.com/YosysHQ/prjtrellis/")
              (commit version)
-             ;; pulls the bitstream db for ECP5 devices; this is useful only by
-             ;; prjtrellis, there is no need to package it separately
+             ;; Pull the bitstream database for ECP5 devices; this is useful
+             ;; only by prjtrellis: there is no need to package it separately.
              (recursive? #t)))
        (file-name (git-file-name name version))
        (modules '((guix build utils)))
@@ -447,9 +447,9 @@ (define-public prjtrellis
     (build-system cmake-build-system)
     (arguments
      (list
-      ;; the examples test directory requires nextpnr, using this package as a
-      ;; backend, which is provided by nextpnr-ecp5; the tests are to be run in
-      ;; there then
+      ;; The examples test directory requires nextpnr, using this package as a
+      ;; backend, which is provided by nextpnr-ecp5: the tests are to be run
+      ;; in this later package.
       #:tests? #f
       #:phases
       #~(modify-phases %standard-phases
@@ -468,7 +468,7 @@ (define-public prjtrellis
     (description
      "Project Trellis is a Nextpnr backend compatible with ECP5 FPGAs.
 The following features are currently available:
-@table @command
+@itemize
 @item logic slice functionality, including carries
 @item distributed RAM inside logic slices
 @item all internal interconnect
@@ -476,8 +476,8 @@ (define-public prjtrellis
 @item block RAM, using inference or manual instantiation
 @item multipliers using manual instantiation
 @item global networks and PLLs
-@item transcievers (DCUs)
-@end table")
+@item transcievers (DCUs.)
+@end itemize")
     (home-page "https://github.com/Yos
--8<---------------cut here---------------end--------------->8---

Pushed as 2ca8d382816.

-- 
Thanks,
Maxim