[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