Hi Matthew, thank you very much for your patch! Below are some comments.
> From 9d400b025a3d60bc29b58ba0bf1301a418ba7aa2 Mon Sep 17 00:00:00 2001 > From: Matthew Jordan <matthewjordandev...@yandex.com> > Date: Wed, 18 May 2016 18:20:30 -0400 > Subject: [PATCH] gnu: Add Tmux themepack > * gnu/packages/tmux.scm: (tmux-themepack): New variable. The first “:” (after “scm”) should be deleted. > @@ -21,10 +22,16 @@ > #:use-module (guix licenses) > #:use-module (guix packages) > #:use-module (guix download) > + #:use-module (guix git-download) > + #:use-module (guix build-system trivial) > #:use-module (guix build-system gnu) > #:use-module (gnu packages) > + #:use-module (gnu packages base) > #:use-module (gnu packages libevent) > - #:use-module (gnu packages ncurses)) > + #:use-module (gnu packages ncurses) > + #:use-module (guix utils) > + #:use-module (guix build utils)) > + This may not be important, but I suggest putting the “use-module” expressions for “(guix utils)” and “(guix build utils)” further up where “(guix packages)” is. > + > +(define-public tmux-themepack > + (package > + (name "tmux-themepack") > + (version "03a3728") ;; No version tags Please see “7.6.3 Version Numbers” in the Guix manual. We use the full commit (in a let binding) and construct a version string from it. > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url "https://github.com/jimeh/tmux-themepack.git";) > + (commit version))) > + (sha256 > + (base32 > + "1d3k87mq5lca042jbap5kxskjy3kg79wjhhpnm6jacbn3anc67zl")))) > + (build-system trivial-build-system) > + (arguments > + `(#:modules ((guix build utils)) > + #:builder (begin > + (use-modules (guix build utils)) > + (let* ((out (string-append > + (assoc-ref %outputs "out") "/share/" ,name > "-" ,version)) > + (env (assoc-ref %build-inputs "coreutils")) > + (in (string-append (assoc-ref %build-inputs > "source")))) > + (copy-recursively in ".") > + (substitute* "themepack.tmux" > + (("#!/usr/bin/env") (string-append "#!" env > "/bin/env"))) > + (mkdir-p out) > + (copy-recursively "." out))))) I wonder: could you use the gnu-build-system, delete the “configure” and “build” phases, and override the “install” phase to copy the files? Instead of running “substitute*” on “themepack.tmux” you can use the “patch-shebang” procedure. > + (propagated-inputs > + `(("tmux" ,tmux) > + ("coreutils" ,coreutils))) Why should coreutils be propagated? And why should tmux be installed automatically? I think “tmux” doesn’t even need to be an input at all. > + (home-page "https://github.com/jimeh/tmux-themepack";) > + (synopsis "Pack of various Tmux themes") How about “Collection of themes for Tmux”. > + (description "A pack of various Tmux themes.") Please add a description with full sentences. Could you please send an updated patch? Thank you! ~~ Ricardo
