Subject: Re: [bug#38811] gnu: Add gnome-menus.

From: Jan Nieuwenhuizen <[email-masked]>
Date: Mon, 30 Dec 2019 14:21:09 +0100

Raghav Gururajan writes:

Hello Raghav,

I was about to commit your patch with changes and then decided there are
a bit too many small things to fix.  In essence your package is fine,
just a a number of cleanups are needed.  Comments in-line below.

Have you tried to running the package; I do not use GNOME?

> From db24eb52caec6097b95d1604adcfeb8a29c72488 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <[email-masked]>
> Date: Mon, 30 Dec 2019 07:47:30 -0500
> Subject: [PATCH] gnu: Add gnome-menus.
>
> * gnu/packages/gnome.scm (gnome-menus). New Variable.

Use lower case on variable: New variable.

> ---
>  gnu/packages/gnome.scm | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index e0712bf99a..607a546768 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -3848,6 +3848,27 @@ It is a basic GtkUIManager replacement based on GAction.  It is suitable for
>  both a traditional UI or a modern UI with a GtkHeaderBar.")
>      (license license:lgpl2.1+)))
>  
> +(define-public gnome-menus
> +  (package
> +    (name "gnome-menus")
> +    (version "3.32.0")
> +    (source (origin
> +	      (method url-fetch)
   ^
Use spaces instead of TABs.

> +	      (uri (string-append "mirror://gnome/sources/gnome-menus/"
> +				  (version-major+minor version) "/gnome-menus-" version ".tar.xz"))

Use spaces instead of TABs, add line break to stay within 80 columns.

> +	      (sha256
> +	       (base32 
                      ^
Remove trailing whitespace.

> +		"0x2blzqrapmbsbfzxjcdcpa3vkw9hq5k96h9kvjmy9kl415wcl68"))))
> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("gettext" ,gettext-minimal)
> +       ("glib" ,glib)
> +       ("pkg-config" ,pkg-config)))
> +    (synopsis "GNOME Menus")

This is too non-descriptive, use something like

"GNOME implementation of the freedesktop menu specification"

> +    (description "It contains the libgnome-menu library, the layout configuration files for the GNOME menu, as well as a simple menu editor. The libgnome-menu library implements the 'Desktop Menu Specification' from freedesktop.org.")

Instead of "It", start with

   GNOME Menus contains ...

add line breaks to stay within 80 columns, use two spaces after each sentence.

> +    (home-page "https://gitlab.gnome.org/GNOME/gnome-menus")
> +    (license license:gpl2+)))

It looks like the package is licensed partly under gpl2 and lgpl2 (not
gpl2+).  Can you please double check?

> +
>  (define-public devhelp
>    (package
>      (name "devhelp")

Can you please send and updated patch?  Most of the corrections are
reported by guix lint, before you send it please run

    ./pre-inst-env guix lint gnome-menus

and make sure there are no errors reported.

Thanks for your contribution!

Greetings,
janneke

-- 
Jan Nieuwenhuizen <[email-masked]> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com