Re: [PATCH] Add nautilus

[Leo Famulari]

On Mon, Feb 01, 2016 at 05:51:33PM +0100, address@hidden wrote:
> Hi,
> 
> i attached patch for review.
> 
> Regards

> From 18bddcf21cac4245997a76d4ea5cfc856d1b6157 Mon Sep 17 00:00:00 2001
> From: Rene Saavedra <address@hidden>
> Date: Mon, 1 Feb 2016 17:45:22 +0100
> Subject: [PATCH] Add nautilus
> 
> ---
>  gnu/packages/nautilus.scm | 63 
> +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 gnu/packages/nautilus.scm

Thanks! I will review some general points. It would be good to get a
GNOME / GTK expert to review this as well — perhaps 宋文武 has some
feedback?

> 
> diff --git a/gnu/packages/nautilus.scm b/gnu/packages/nautilus.scm
> new file mode 100644
> index 0000000..f982f0b
> --- /dev/null
> +++ b/gnu/packages/nautilus.scm
> @@ -0,0 +1,63 @@
> +2016-02-01  Rene Saavedra  <address@hidden>
> +
> +     *  gnu/packages/nautilus.scm: New file.
> +     *  gnu-system (GNU_SYSTEM_MODULES): Add it.

The above lines should go in the commit message, rather than in the
patch itself. You can then generate the patch with `git format-patch`.

> +
> +(define-module (gnu packages nautilus)
> +  #:use-module ((guix licenses) #:prefix license:)
> +  #:use-module (guix packages)
> +  #:use-module (guix download)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix utils)
> +  #:use-module (gnu packages freedesktop)
> +  #:use-module (gnu packages glib)
> +  #:use-module (gnu packages gnome)
> +  #:use-module (gnu packages gtk)
> +  #:use-module (gnu packages photo)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages xml)
> +  #:use-module (gnu packages xorg))

Please insert a blank line here.

> +(define-public nautilus
> +  (package
> +    (name "nautilus")
> +    (version "3.18.2")
> +    (source
> +     (origin
> +      (method url-fetch)
> +      (uri (string-append "mirror://gnome/sources/" name "/"
> +                          (version-major+minor version) "/"
> +                           name "-" version ".tar.xz"))
> +      (sha256
> +       (base32
> +        "0jj23n8vmmyc4gp5xhiz7slsxwksydp26blxi5m154yaw9lgdp38"))))
> +    (build-system gnu-build-system)

This is really a question for a GNOME user, but I wonder if it would be
appropriate to use the glib-or-gtk-build-system?

> +    (arguments
> +     `(#:configure-flags
> +       (list "--disable-tracker" "--disable-packagekit")

It's a good idea to explain package-specific configure-flags, in my
opinion.

> +       #:tests? #f))

Please add a note explaining why tests are disabled. If there are no
tests, just say something like "no test suite".

> +    (native-inputs
> +     `(("glib" ,glib "bin")

Does glib need to be in both native-inputs and inputs?

> +       ("gobject-introspection" ,gobject-introspection)
> +       ("intltool" ,intltool)
> +       ("pkg-config" ,pkg-config)))
> +    (inputs
> +     `(("adwaita-icon-theme" ,adwaita-icon-theme)

I think our policy is to have users install the icons into their
profile, and let other packages find them through environment variables,
as explained here:
http://lists.gnu.org/archive/html/guix-devel/2016-01/msg00764.html
Is that appropriate in this case?

> +       ("exempi" ,exempi)
> +       ("glib" ,glib)
> +       ("gnome-desktop" ,gnome-desktop)
> +       ("gsettings-desktop-schemas" ,gsettings-desktop-schemas)
> +       ("gtk+" ,gtk+)
> +       ("gvfs" ,gvfs)
> +       ("libexif" ,libexif)
> +       ("libnotify" ,libnotify)
> +       ("libx11" ,libx11)
> +       ("libxml2" ,libxml2)
> +       ("pango" ,pango)))
> +    (home-page "https://wiki.gnome.org/Apps/Nautilus";)
> +    (synopsis "GNOME file manager")
> +    (description
> +     "Nautilus is a file manager designed for the GNOME desktop.
> +Nautilus design provides the user a simple way and elegant to
> +managing files and applications, through simple menus and
> +notifications.")

I haven't used Nautilus before. Can someone else weigh in on the
description?

> +    (license license:gpl2+)))  ; libnautilus-extenstions under lgpl

You can provide list of licenses. There is an example in the package
definition for 'ruby-power-assert'. But please keep the comment about
what parts are covered under what license.

> -- 
> 2.6.3
>