Subject: Re: [bug#58162] [PATCH] gnu: Add qdmr

From: Ryan Tolboom <[email-masked]>
Date: Thu, 29 Sep 2022 17:20:16 -0400

Hello,

Thank you very much for your comments! I apologize for my brevity, I'm
still trying to work out the whole 'git send-email' workflow so my
patches arrive without comments. I did leave some comments through the
web interface, but I'm not seeing them show up.

I was using the 'Hello World' package example initially:
https://guix.gnu.org/cookbook/en/html_node/A-_0060_0060Hello-World_0027_0027-package.html
Do you think I should submit a patch to move home-page to the correct
place in that example?

Regarding the description, I was on the fence about it. I took it from
the GitHub page for the project and wasn't quite sure if it was too
wordy. Also regarding mentioning services in the description, several
other packages in radio.scm do it as well: rtl-sdr, airspyhf, hackrf,
and rfcat. I actually found it quite useful when setting things up,
does it have to be removed?

Anyway the new patch implements all your changes, which BTW
g-expressions are really neat, with the exception of removing the
service line from the description.

Let me know your thoughts,

Ryan

On Thu, Sep 29, 2022 at 1:51 PM ( <[email-masked]> wrote:
>
> Hey!
>
> On Thu Sep 29, 2022 at 2:57 AM BST, Ryan Tolboom wrote:
> > +    (native-inputs (list qttools-5))
> > +    (inputs (list qtbase-5 qtserialport qtlocation yaml-cpp libusb))
>
> Usually inputs and native-inputs come after arguments.
>
> > +    (arguments
> > +     `(#:tests? #f ;No tests
> > +       #:phases (modify-phases %standard-phases
> > +                  (add-after 'unpack 'fix-paths
> > +                    (lambda* (#:key outputs #:allow-other-keys)
> > +                      (substitute* "lib/CMakeLists.txt"
> > +                        (("DESTINATION \"/etc/udev/")
> > +                         (string-append "DESTINATION \""
> > +                                        (assoc-ref outputs "out") "/lib/udev/"))))))))
>
> Try using the new gexp style for arguments:
>
>   (arguments
>    (list #:tests? #f ;no tests
>          #:phases
>          #~(modify-phases %standard-phases
>              (add-after 'unpack 'patch-paths
>                (lambda _
>                  (substitute* "lib/CMakeLists.txt"
>                    (("(DESTINATION \")/etc/udev/" _ directive)
>                     (string-append directive #$output "/lib/udev/"))))))))
>
> > +    (description
> > +     "qdmr is a graphical user interface (GUI) application that allows one to
> > +program several types of DMR radios.  To this end, it aims at being a more
> > +universal codeplug programming software (CPS) compared to the device and even
> > +revision specific CPSs provided by the manufacturers.  The goal of this project
> > +is to provide a single, comfortable, well-documented and platform-independent
> > +CPS for several types of (mainly Chinese) DMR radios.
> > +
> > +To install the qdmr udev rules, you must extend @code{udev-service-type} with this
> > +package.  E.g.: @code{(udev-rules-service 'qdmr qdmr)}")
>
> IMO this is too long and markety :) Also, I don't think you should mention
> services in a package description.
>
> > +    (home-page "https://dm3mat.darc.de/qdmr/")
>
> Home-page usually goes directly before synopsis.
>
>     -- (

-- 
Ryan Tolboom
Using Tech LLC