[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#41083] gnu: xfe: Fix hard-coded fhs directories.
From: |
Ludovic Courtès |
Subject: |
[bug#41083] gnu: xfe: Fix hard-coded fhs directories. |
Date: |
Mon, 04 May 2020 23:04:48 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Hello!
Raghav Gururajan <address@hidden> skribis:
>>From 660f134e15438e7ee7aec1c076dca93c68e4edc6 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <address@hidden>
> Date: Mon, 4 May 2020 13:07:02 -0400
> Subject: [PATCH] gnu: xfe: Fix hard-coded fhs directories.
>
> * gnu/packages/disk.scm (xfe): Fix hard-coded fhs directories.
> [arguments]<#:phases>['patch-xfe-paths]: Delete phase.
> [arguments]<#:phases>['patch-bin-dirs]: New phase.
> [arguments]<#:phases>['patch-share-dirs]: New phase.
> [inputs]<bash,coreutils,file,findutils>: New inputs.
Nitpick: You don’t need to mention #:phases above, and the angle
brackets are inappropriate for inputs. See ‘git log’ for examples.
> - (native-inputs
> - `(("intltool" ,intltool)
> - ("pkg-config" ,pkg-config)))
> - (inputs
> - `(("fox" ,fox)
> - ("freetype" ,freetype)
> - ("x11" ,libx11)
> - ("xcb" ,libxcb)
> - ("xcb-util" ,xcb-util)
> - ("xft" ,libxft)
> - ("xrandr" ,libxrandr)))
To reduce review time :-), it’s a good idea to avoid unnecessary
changes. In this case, you should avoid moving things around because
that makes the patch harder to read.
> + (substitute* "src/FilePanel.cpp"
> + (("/bin/sh") sh)
> + (("/usr/bin/du") du)
> + (("/usr/bin/sort") sort)
> + (("/usr/bin/cut") cut)
> + (("/usr/bin/xargs") xargs))
> + (substitute* "src/help.h"
> + (("/bin/sh") sh)
> + (("/bin/ls") ls))
> + (substitute* "src/SearchPanel.cpp"
> + (("/usr/bin/du") du)
> + (("/usr/bin/sort") sort)
> + (("/usr/bin/cut") cut)
> + (("/usr/bin/xargs") xargs))
> + (substitute* "src/startupnotification.cpp"
> + (("/bin/sh") sh))
> + (substitute* "src/xfeutils.cpp"
> + (("/usr/bin/file") file))
I think you can just define a variable like:
(coreutils (assoc-ref inputs "coreutils"))
and then use (string-append coreutils "/bin/sort") and so on, instead of
defining many variables that have a single user.
> (let*
> - ((out (assoc-ref outputs "out"))
> - (share (string-append out "/share"))
> - (xferc (string-append out "/share/xfe/xferc"))
> - (xfe-theme (string-append out
> "/share/xfe/icons/xfe-theme")))
> - ;; Correct path for xfe registry.
> + ((out
> + (assoc-ref outputs "out"))
> + (share
> + (string-append out "/share"))
> + (xfe
> + (string-append out "/share/xfe"))
Avoid the indentation changes (previous version was fine, although we
usually start the list of bindings on the same line as ‘let*’).
> - ;; Correct path for xfe icons.
> - (substitute* "src/xfedefs.h"
> - (((string-append
> - "~/.config/xfe/icons/xfe-theme:"
> - "/usr/local/share/xfe/icons/xfe-theme:"
> - "/usr/share/xfe/icons/xfe-theme"))
> - xfe-theme))
The ~/.config/xfe bit is going away, right?
Could you send an updated patch?
Thanks,
Ludo’.