https://bugzilla.redhat.com/show_bug.cgi?id=967659 Michael Schwendt <mschwendt@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mschwendt@xxxxxxxxx --- Comment #4 from Michael Schwendt <mschwendt@xxxxxxxxx> --- * Try to do a self-review of your package with the help of the following page: https://fedoraproject.org/wiki/Packaging:ReviewGuidelines [ https://fedoraproject.org/wiki/Join_the_package_collection_maintainers ] [ https://fedoraproject.org/wiki/Category:Package_Maintainers ] * Run "rpmlint -I" on all packages, the src.rpm *and* the built rpms. Apply fixes for obvious errors/warnings, ignore false positives, preferably comment on what rpmlint reports. * A brief look at the spec file: > Summary: Free journal software for everyone Summary: Cross-platform journal/diary tool would do a better job at summing up what the package offers. > BuildRequires: qt, qt-assistant, qt-mysql, qt-devel, qt-webkit, > qt-webkit-devel, patch 1) "patch" is available in the minimum build environment already and need not be specified as BuildRequires: https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 2) Double-check the others for redundancies. At least "qt" and "qt-webkit" will be available as dependencies of qt-devel and qt-webkit-devel already. > # Apply standard Fedora patch so the app compiles properly > patch Makefile < fedora_build.patch Hmmm, this is fragile. First of all, it would be more normal to apply patches in %prep (after %setup) via %patchX and to add them as PatchX tags in the spec file. But since the patch file is included in the tarball, applying it manually is acceptable. Secondly, applying a patch manually in %build like it is done here breaks --short-circuit -bc rpmbuilds. So, apply patches in %prep. > Requires: qt, qt-assistant, qt-mysql, qt-webkit https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires > make https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make > strip robojournal Don't strip any files yourself. That breaks generation of -debuginfo packages. rpmlint will notice that, too. https://fedoraproject.org/wiki/Packaging:Guidelines#Debuginfo_packages > #install files manually because "make install" doesn't work > with rpmbuild in this case. _What_ "doesn't work"? > %post > mandb -p To be deleted. Packages don't do that. > %clean > make distclean To be deleted. "make distclean" is also not what %clean is used for. https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean > %{_datadir}/applications/robojournal.desktop You need to _use_ desktop-file-utils inside the spec file when packaging .desktop files: https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files > %{_datadir}/menu/robojournal The directory /usr/share/menu doesn't exist yet in Fedora filesystem packages, and this package doesn't include it either. What is it used for? Isn't it specific to Debian based systems? $ repoquery --whatprovides /usr/share/menu luckybackup-0:0.4.7-3.fc19.x86_64 > %{_mandir}/man7/robojournal.7.gz %{_mandir}/man7/robojournal.7* would be cleaner, since it allows for a changed/customised/dropped compression technique. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=dIeJaVKy28&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review