[bug#40994] patch#40994 Programs With Movie Titles (PWMT)

[Marius Bakke <mbakke@fastmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3182 bytes --]

Hi!

Raghav Gururajan <raghavgururajan@disroot.org> writes:

> Here, I have attached another patch-set for modifying stuffs. :-)

I have added another set of nit-picks!  :-)

> From 30042d73a58f90971cd6c37bf56269bd3abb2533 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan@disroot.org>
> Date: Sat, 2 May 2020 22:28:44 -0400
> Subject: [PATCH 08/14] gnu: girara: Update package definition.
>
> * gnu/packages/pwmt.scm (girara):
> [source]<origin>[method]: Changed from git-fetch to url-fetch; and
> remove file-name field.
> [arguments]<#:glib-or-gtk?>: New argument.
> [arguments]<#:configure-flags>[-Dnotify]: New flag.
> [native-inputs]<doxygen>: New input.
> [inputs]<glib,gtk+,json-c,libnotify,pango>: New inputs.
> [propagated-inputs]<gtk+>: Removed.
> [synopsis]: Updated.
> [description]: Updated.

I know it's a lot to ask, but it would be great if you could split this
up in multiple patches, one per logical change.  I.e. this one patch
would be better as a series like:

Raghav Gururajan (7):
  gnu: girara: Download tarball instead of git source.
  gnu: girara: Wrap with Glib variables.
  gnu: girara: Add notification support.
  gnu: girara: Build and install documentation.
  gnu: girara: Do not propagate GTK+.
  gnu: girara: Enable more features.
  gnu: girara: Update synopsis & description.

That makes it possible to revert some of these changes in case of
problems without undoing the whole thing, and also makes reviewing
easier.

I'm also skeptical about some of these (why is #:glib-or-gtk? necessary
for this library, why does GTK+ no longer need to be propagated, and
what are all those new inputs for?).  By lumping everything together
it's difficult to reason about these changes.

> From 7e3558dda412d33fffb7bb0668886f1ede3d14c8 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan@disroot.org>
> Date: Sat, 2 May 2020 23:29:28 -0400
> Subject: [PATCH 09/14] gnu: zathura: Update package definition.
>
> * gnu/packages/pwmt.scm (zathura):
> [arguments]<#:glib-or-gtk?>: New argument.
> [native-inputs]<desktop-fileutils,doxygen,python-breathe,python-
> sphinx-rtd-theme>: New inputs.

Same here, what do these inputs do?

> [inputs]<appstream-glib,cairo,file,girara,glib,json-c,gtk+,libnotify,
> libseccomp>: New inputs.

And these?

> [propagated-inputs]<cairo,girara>: Removed.
> [synopsis]: Updated.
> [description]: Updated.

ISTM this could be split into at least four different patches.

> From 345a2b2ffc04c99fdfc3785ac6d19f053afd1b90 Mon Sep 17 00:00:00 2001
> From: Raghav Gururajan <raghavgururajan@disroot.org>
> Date: Sat, 2 May 2020 23:42:49 -0400
> Subject: [PATCH 10/14] gnu: zathura-ps: Update package definition.
>
> * gnu/packages/pwmt.scm (zathura-ps):
> [arguments]<#:glib-or-gtk?>: New argument.

Why does this plugin package need #:glib-or-gtk?.

> [inputs]<cairo,girara,glib,gtk+,json-c,libnotify>: New inputs.

It's strange that all of these packages require almost the exact same
set of inputs.  Perhaps they should be propagated somewhere?

> [synopsis]: Updated.
> [description]: Updated.

This should also be a separate patch.

I think you catch my drift here, can you send an updated series?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]