[Bug 3863] Review request: game-data-packager - Installer for game data files

[RPM Fusion Bugzilla]

https://bugzilla.rpmfusion.org/show_bug.cgi?id=3863

Hans de Goede <j.w.r.degoede at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |j.w.r.degoede at gmail.com
             Blocks|2                           |3
         AssignedTo|rpmfusion-package-review at rp |j.w.r.degoede at gmail.com
                   |mfusion.org                 |

--- Comment #3 from Hans de Goede <j.w.r.degoede at gmail.com> 2015-12-27 13:07:17 CET ---
Hi,

Sorry for being slow in reviewing this. It has been a bit busy lately, but I've
time now.

Overall the spec file looks good, a few remarks:

1) You mix Recommends and Suggests, according to:

https://fedoraproject.org/wiki/PackagingDrafts/WeakDependencies

Suggests are pretty much ignored under Fedora atm. So you may want to change
most Suggests into Recommends, at least for those Suggests which are available
as Fedora packages in the standard Fedora repos.

2) The %install section is quite long, you say that you've added a "make
install" target upstream, it would be good to use this in the next version

3) The "%clean" section is obsolete, and also it is not intended to do a "make
clean" but to remove the $RPM_BUILD_ROOT aka the "make install" DESTDIR.

4) I see no license files in the %files sections. I guess since this comes from
Debian that it relies on the shared versions of the GPL text used in Debian, in
Fedora we ship a single license file per package, it would be good if you could
add the GPLv2 txt to the upstream tarbal (typically called COPYING) and add a 
"%license COPYING" to the file-list for both the main and the
doom2-masterlevels packages.

Regards,

Hans

-- 
Configure bugmail: https://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
You are the assignee for the bug.