guix-patches
[Top][All Lists]
Advanced

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

[bug#40752] gnu: Add eject.


From: Tobias Geerinckx-Rice
Subject: [bug#40752] gnu: Add eject.
Date: Wed, 22 Apr 2020 00:28:30 +0200

Raghav,

Thanks for the patch! Amazing that this wasn't in Guix yet. Even more amazing that I didn't package it.

Raghav Gururajan 写道:
+(define-public eject
+  (package
+    (name "eject")
+    (version "2019.08.22")

You've been around long enough to know how we package git releases :-) Certainly not like this.

More like

 ;; Comment explaining why we use a git snapshot.
 (let ((commit "12a22e…")
       (revision "0"))
   (package …
     (version (git version "2.1.…" revision commit))

please.

Upstream[0 ]says there's a 2.1.6 version, but it may exist nowhere but in the author's mind. If you do find a tarball, tag, or corresponding commit, use it, or explain in a comment why 12a22e is better.

+    (source
+     (origin
+       (method git-fetch)
+       (uri
+        (git-reference
+         (url "https://git.code.sf.net/p/eject/git";)
+         (commit "12a22e")))

Please don't abbreviate commits like this.

+       (file-name (git-file-name name version))

Yay!

+       (sha256
+ (base32 "1dxj0zn32l8k16j5innh8y4snambx1b702ny37114drddqcqac6h"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; No target

Write ‘; no test suite’ instead, which is to be expected from a package like this.

‘No target’ makes it sound like who knows, there might be tests, but there's no make rule to run them, so we don't. Not a good look.

+       #:make-flags
+       (list

Hm?  Eh?  Oh…  Please insert a comment here:

;; ./configure respects --prefix=; the handwritten po/Makefile doesn't.

+        (string-append "PREFIX=" (assoc-ref %outputs "out")))))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("automake" ,automake)
+       ("gettext" ,gettext-minimal)))
+    (inputs
+     `(("lvm2" ,lvm2)
+       ("sg3-utils" ,sg3-utils)))
+    (synopsis "Utility for Removable Media")

…is too vague.  And this is worse:

+ (description "Eject allows removable media (typically a CD-ROM, floppy disk, +tape, or JAZ or ZIP disk) to be ejected under software control. The command can +also control some multi-disc CD-ROM changers, the auto-eject feature supported +by some devices, and close the disc tray of some CD-ROM drives. ")

I know this is upstream's handiwork; not yours, but crikey.

Always feel free to edit the hell out of these texts. ‘Software Foo allows gizmos to be frobbed under software control’? → ‘Foo frobs gizmos’. Done.

A quick attempt with some inspiration from Debian:

 (synopsis "Eject and safely disconnect removable media")
 (description
"This little program will eject CD-ROMs and other ejectable removable media. It can also set the auto-eject feature, close the tray of some CD-ROM drives,
and select the active disc in supported multi-disc changers.

You can also use @command{eject} to properly disconnect external mass-storage devices like mobile phones, digital cameras, or portable music players.")

+    (home-page "http://eject.sourceforge.net";)
+    (license license:gpl2+)))
+

I wasn't able to apply & test this patch; I guess ‘udevil’ is still under review. I don't have time to review it as well.

Please: try to add new packages in alphabetical order, NOT blindly to the beginning or end of files. For once, this has nothing (well, less) to do with my obsession for ordnung. It makes annoying issues like this less likely.

It doesn't matter that most package .scm files are an unordered mess: if you'd added eject near the beginning and udevil near the end, the two unrelated packages wouldn't conflict.

Kind regards,

T G-R

[0]: https://sourceforge.net/p/eject/git/ci/master/tree/NEWS

Attachment: signature.asc
Description: PGP signature


reply via email to

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