* Re: Digimend kernel drivers for Guix
2019-05-28 14:04 Digimend kernel drivers for Guix Marlin
@ 2019-05-28 18:25 ` Tobias Geerinckx-Rice
2019-05-28 18:33 ` Tobias Geerinckx-Rice
0 siblings, 1 reply; 5+ messages in thread
From: Tobias Geerinckx-Rice @ 2019-05-28 18:25 UTC (permalink / raw)
To: guix-devel, Marlon
[-- Attachment #1: Type: text/plain, Size: 4624 bytes --]
Marlin,
Marlin wrote:
> I have just managed to get the digimend kernel drivers packaged
> for guix:
Cool! I encourage you to read the ‘Contributing’ section of the
manual, set up your own git repository, and submit a patch adding
this package to linux.scm.
I don't mind merging this version eventually, but you'll have to
do so sooner or later if you want to keep contributing (and I hope
you will). Might as well jump in.
> (define-public digimend-kernel-drivers
I realise this is the upstream name. I'm not sure what to do
here.
We currently have 2 separately packaged Linux kernel modules:
vhba-module (which was first) and acpi-call-linux-module (the
latter added by me). I'd really like to see some semblance of
consistency in naming these things and don't think ‘-module[s]’
(for Guile?), ‘-driver[s]’ (not all modules are drivers, but I
agree it's subtle), or ‘-kernel-’ (which kernel?) cut it.
> (package
> (name "digimend-kernel-drivers")
> (version "9")
> (source (origin
> (method url-fetch)
> (uri (string-append "[…long URL is long…]"
Please keep lines shorter than 80 characters. Often indenting can
make the difference (SOURCE and ORIGIN here could be on two
separate lines, for example) but that won't help you here.
You're already calling STRING-APPEND, so you can simply split up
the URL:
(uri (string-append
"https://github.com/DIGImend/digimend-kernel-drivers/"
"releases/download/v9/digimend-kernel-drivers-"
…
…or however you prefer. Also note the hard-coded ‘9’ that needs
replacing by VERSION.
> (build-system linux-module-build-system)
All lines from here on are misleadingly indented: this opening
bracket should be even with that of SOURCE above.
If you use emacs, fixing this is as easy as running C-M-q at the
beginning of the package definition. If you don't (nobody's
perfect), we provide a stand-alone (emacs) script to do this:
./etc/indent-code.el gnu/packages/FILE.scm PACKAGE
…although I've never used it. See the ‘Formatting Code’ section
of the manual.
> (arguments '(#:phases (modify-phases %standard-phases
> (delete 'check))))
This works (and once re-indented it won't even exceed 80
characters), but consider the more conventional and easy-to-skim
(arguments
'(#:phases
(modify-phases %standard-phases
(delete 'foo)))) ; no foo gizmo
…including the helpful comment.
However, in this case, please use the even simpler:
(arguments
'(#:tests?)) ; no test suite
> (synopsis "Kernel Drivers for non-wacom Digital
> Tablets")
‘Drivers’ & ‘Digital Tablets’ shouldn't be capitalised; ‘Wacom’
should.
However, I think the project already offers a good synopsis on
their home page: how about "Linux drivers for generic graphics
tablets"?
> (description "Provides Drivers for Tablets from \
> companies like huion and ugee.")
Same here: nouns in English don't need to be capitalised, proper
nouns such as ‘Huion’ do (even if they don't do so themselves!).
Why single out these two manufacturers? The home page lists a lot
more.
This is also far to short to be a description: ideally, they
should be around ~10 lines. Here too, the home page has some good
stuff ready to use.
The DIGImend project aims at improving Linux support for generic
graphics tablets. Essentially, this means any USB-connected
drawing
tablet not produced by Wacom (and often more affordable).
This package supports tablets designed and sold by Huion, KYE,
UC-Logic and Waltop, rebranded and sold by Aiptek, Genius,
Monoprice,
Princeton, Trust, and many others.
This way, people searching for ‘Aiptek’ will still find this
package, and I took the liberty of ‘keyword-stuffing’ the word
‘drawing’ in there for good measure ;-)
This is still pretty short. If there's more to be said, please
add it!
> (license gpl2)))
This seems to be accurate. You'll need to add a ‘license:’ when
moving this to linux.scm.
Thank you for packaging this, and I hope you'll submit a
git-formatted patch to guix-patches@ soon. If you get stuck,
don't hesitate to ask questions on guix-devel@ or #guix.
Kind regards,
T G-R (nckx)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread