| Summary: | Review Request: naev - 2d action, RPG space game | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jonathan Dieter <jdieter> |
| Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | fedora-package-review, notting, tcallawa |
| Target Milestone: | --- | Flags: | tcallawa:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | naev-data-0.5.0-4.fc15 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2011-07-08 18:07:13 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Bug Depends On: | 711048 | ||
| Bug Blocks: | |||
|
Description
Jonathan Dieter
2011-06-06 11:25:01 UTC
The data files are at bug #711048 *** Bug 713925 has been marked as a duplicate of this bug. *** Quick comments:
* You do not need the BuildRoot setting, it is obsolete in all current versions of Fedora. It is only needed for EPEL branches older than 6.
* You do not need the rm -rf %{buildroot} at the beginning of %install. It is the default in all current versions of Fedora. It is only needed for EPEL branches older than 6.
* You do not need the default %clean section. A %clean that simply deletes the %{buildroot} is the default in all current versions of Fedora. It is only needed for EPEL branches older than 6.
* configure seems to be searching for libGL and libGLU, perhaps mesa-libGL-devel, mesa-libGLU-devel should be added as BuildRequires?
* make DESTDIR=%{buildroot} install seems to work fine, perhaps you should use it (and just run desktop-file-validate %{buildroot}%{_datadir}/applications/%{name}.desktop)
* You do not need to explicitly mark manpages as %doc, anything in the mandir is automatically marked as %doc.
(In reply to comment #3) > Quick comments: > > * You do not need the BuildRoot setting, it is obsolete in all current versions > of Fedora. It is only needed for EPEL branches older than 6. Fixed > * You do not need the rm -rf %{buildroot} at the beginning of %install. It is > the default in all current versions of Fedora. It is only needed for EPEL > branches older than 6. Fixed > * You do not need the default %clean section. A %clean that simply deletes the > %{buildroot} is the default in all current versions of Fedora. It is only > needed for EPEL branches older than 6. Fixed > * configure seems to be searching for libGL and libGLU, perhaps > mesa-libGL-devel, mesa-libGLU-devel should be added as BuildRequires? Fixed > * make DESTDIR=%{buildroot} install seems to work fine, perhaps you should use > it (and just run desktop-file-validate > %{buildroot}%{_datadir}/applications/%{name}.desktop) Fixed, though I now manually choose the highest quality png in extras/logos as the icon. The default png is 32x32, which looks pretty bad in gnome-shell. > * You do not need to explicitly mark manpages as %doc, anything in the mandir > is automatically marked as %doc. I tried this, but got: Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/jonathan/rpmbuild/BUILDROOT/naev-0.5.0-2.fc15.i386 error: Installed (but unpackaged) file(s) found: /usr/share/man/man6/naev.6.gz RPM build errors: Installed (but unpackaged) file(s) found: /usr/share/man/man6/naev.6.gz Maybe I'm just making a stupid mistake? Updated packages at: Spec URL: http://www.lesloueizeh.com/jdieter/naev.spec SRPM URL: http://www.lesloueizeh.com/jdieter/naev-0.5.0-2.fc15.src.rpm 32-bit F15 RPM: http://www.lesloueizeh.com/jdieter/naev-0.5.0-2.fc15.i686.rpm Huh. The %{_mandir}/man6/* line should be correct, what was unnecessary was the %doc in front of it.
Good:
- rpmlint checks return clean
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv3) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (cbaa09d036188a22c0c9c70f1db6e00923b138ff897cdd37072adf4a438a503d)
- package compiles on F-15 (http://koji.fedoraproject.org/koji/taskinfo?taskID=3164092)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- .desktop file OK
APPROVED.
Updated spec to remove defattr. And, yeah, that was my stupid mistake on the whole %{_mandir} thing. I just removed the whole thing, and then put it back without the %doc, without realizing that's what I did.
Spec URL:
http://www.lesloueizeh.com/jdieter/naev.spec
SRPM URL:
http://www.lesloueizeh.com/jdieter/naev-0.5.0-3.fc15.src.rpm
32-bit F15 RPM:
http://www.lesloueizeh.com/jdieter/naev-0.5.0-3.fc15.i686.rpm
New Package SCM Request ======================= Package Name: naev Short Description: 2d action, RPG space game Owners: jdieter Branches: f14 f15 el6 InitialCC: Git done (by process-git-requests). naev-data-0.5.0-4.fc15,naev-0.5.0-3.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/naev-data-0.5.0-4.fc15,naev-0.5.0-3.fc15 Spot, thanks much for this review! Naev has been built for F14, F15 and EL6 now. naev-data-0.5.0-4.fc15, naev-0.5.0-3.fc15 has been pushed to the Fedora 15 testing repository. naev-data-0.5.0-4.fc15, naev-0.5.0-3.fc15 has been pushed to the Fedora 15 stable repository. |