unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Jan Nieuwenhuizen <janneke@gnu.org>
To: Raghav Gururajan <raghavgururajan@disroot.org>
Cc: 38811@debbugs.gnu.org
Subject: [bug#38811] gnu: Add gnome-menus.
Date: Mon, 30 Dec 2019 14:21:09 +0100	[thread overview]
Message-ID: <87d0c6hrui.fsf@gnu.org> (raw)
In-Reply-To: <c1e9146e408b095dd570157af34bf7d9@disroot.org> (Raghav Gururajan's message of "Mon, 30 Dec 2019 12:55:05 +0000")

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 <raghavgururajan@disroot.org>
> 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 <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.com

  reply	other threads:[~2019-12-30 13:22 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-30 12:52 [bug#38811] gnu: Add gnome-menus Raghav Gururajan
2019-12-30 12:55 ` Raghav Gururajan
2019-12-30 13:21   ` Jan Nieuwenhuizen [this message]
2019-12-30 18:41     ` Raghav Gururajan
2019-12-30 20:02       ` bug#38811: " Jan Nieuwenhuizen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d0c6hrui.fsf@gnu.org \
    --to=janneke@gnu.org \
    --cc=38811@debbugs.gnu.org \
    --cc=raghavgururajan@disroot.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).