guix-devel
[Top][All Lists]
Advanced

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

Re: submit package vinagre


From: Ricardo Wurmus
Subject: Re: submit package vinagre
Date: Sun, 24 Jan 2016 16:43:03 +0100
User-agent: mu4e 0.9.13; emacs 24.5.1

Hi,

> i create a definition package for vinagre program. Can you review and 
> return comments?

thanks for making the effort to package vinagre.

Normally, we expect contributions to be sent in patch format.  The best
way to do this is to clone the git repository, perform changes there,
commit them with appropriate summaries, and then use “git format-patch
-1”.  The file that is produced by the last command can then be attached
to an email.

For each new variable/package we usually have one commit/patch.

Following are some comments about the file you sent.

* All modules should have a license header.  You can adapt this from
  any of the other modules.

* The module definition declares to use “(guix build-system
  glib-or-gtk)”, but none of the packages you defined use this build
  system.  Are all the modules you import there really needed?

> (define-public gtk-vnc
>   (package
>     (name "gtk-vnc")
>     (version "0.5.4")
>     (source
>      (origin
>       (method url-fetch)
>       (uri (string-append 
> "http://ftp.gnome.org/pub/gnome/sources/gtk-vnc/0.5/";

This line is too long.  You can split the string, but even better: you
can probably use “mirror://gnome/sources” here.

>     (home-page "http://www.gnome.org";)

Is there no separate project webpage?

>     (synopsis
>      "gtk-vnc")

“guix lint” would probably complain about this.  A synopsis should be
more than just the name.  It also should fit on one line.  Something
like “VNC viewer widget for GTK” maybe.

>     (description
>      "gtk-vnc is a VNC viewer widget for GTK.")

That’s a little short.

>     (license license:lgpl3)))

Only this version or is any later version of this license accepted?

> (define-public vinagre
>   (package (inherit gtk-vnc)

There is no good reason to inherit from “gtk-vnc”.

>     (name "vinagre")
>     (version "3.18.2")
>     (source
>      (origin
>       (method url-fetch)
>       (uri (string-append 
> "http://ftp.gnome.org/pub/gnome/sources/vinagre/3.18/";
>                           name "-" version ".tar.xz"))

Same as above: please use “mirror://gnome” here, and make sure the line
isn’t overly long.

>       (sha256
>        (base32
>         "1i7v90zw1s7526qx7b5pxzaray1l9wqxam2n7r1sjx8bvsci5f35"))))
>     (build-system gnu-build-system)
>     (inputs
>      `(("gnome-desktop" ,gnome-desktop)
>        ("gsettings-desktop-schemas" ,gsettings-desktop-schemas)
>        ("libgnome-keyring" ,libgnome-keyring)
>        ("adwaita-icon-theme" ,adwaita-icon-theme)
>        ("gtk+" ,gtk+)
>        ("icon-naming-utils" ,icon-naming-utils)
>        ("libxml2" ,libxml2)
>        ("libsecret" ,libsecret)
>        ("gtk-vnc" ,gtk-vnc)
>        ("avahi" ,avahi)))
>     (native-inputs
>      `(("pkg-config" ,pkg-config)
>        ;("python" ,python-2)

Why is this commented?

>        ("intltool" ,intltool)
>        ("itstool" ,itstool)
>        ("glib:bin" ,glib "bin")))
>     (home-page "http://www.gnome.org";)

Does Vinagre have its own project home page?  How about
https://wiki.gnome.org/Apps/Vinagre instead?

>     (synopsis
>      "Vinagre")

We need a real synopsis here...

>     (description
>      "Vinagre is a remote desktop viewer for GNOME.")

...and something more descriptive here.

>     (license license:lgpl3)))

Is this license correct?  Most files are licensed under GPLv2+, as far
as I can see (even though COPYING contains the text of the GPLv3).

~~ Ricardo




reply via email to

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