guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#56140] [Patches] Add and update music packages


From: Alice BRENON
Subject: [bug#56140] [Patches] Add and update music packages
Date: Tue, 28 Jun 2022 09:45:26 +0200

Hi !

Thanks for submitting this patch ! I'm in no position to provide
a definitive answer to accept or reject this but here are some immediate
things that could be improved.

- your description of Fabla doesn't add much information compared to
  the synopsis. What makes it special ? How is it different from the
  other similar tools ? (see
  
https://guix.gnu.org/fr/manual/devel/en/html_node/Synopses-and-Descriptions.html#Synopses-and-Descriptions
  for inspiration)
- the other packages seem to have more extensive description, but they
  make very long lines, have you tried formatting your package using
  the `guix style` command ?
  
(https://guix.gnu.org/fr/manual/devel/en/html_node/Formatting-Code.html#Formatting-Code)
- you seem to consistently use the "old" syntax for
  packages in native-inputs, as opposed to inputs, is there a good
  reason for that or have you maybe only found examples where the
  native-inputs haven't been modernized ? While it makes sense to
  rename `cmake-minimal` to `cmake` in the definition of
  `distrho-ports` (though I wonder `luppp` didn't require the same
  renaming and keeps the key "cmake-minimal" for `cmake-minimal`, all
  the others occurrences (`("pkg-config" ,pkg-config)`, `("lv2" ,lv2)`…)
  are totally unneeded for as far as I understand and could be
  simplified as `(list pkg-config lv2 mesa)`.

I'm also a bit curious as to why tests need to be explicitly
deactivated in `sorcer`, does running the default cmake command to test
the project result in a failure, not just a NOP ?

I hope some other people will take it over from there because I don't
have much more to contribute : )

Cheers,

Alice





reply via email to

[Prev in Thread] Current Thread [Next in Thread]