Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=769697 Jussi Lehtola <jussi.lehtola@xxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+ --- Comment #9 from Jussi Lehtola <jussi.lehtola@xxxxxx> 2012-01-04 15:58:05 EST --- Much better. ** You don't need to use both desktop-file-install and desktop-validate; the latter is used when the desktop file is installed by, e.g., "make install". ** What does patch1 do? It should be documented in the spec file. ... I see now that the patch removes the shebang from the txt2tags.py file. In this case, the patch should be named, e.g., nested-1.2.2-shebang.patch. The purpose is twofold: first, it identifies which version the patch was written for, and second, it identifies what the patch does. Patches are usually prefixed by the name of the package, since in the old days all sources were in the same directory (but this is of course no longer the case). ** Same goes for sources without source URLs ( http://fedoraproject.org/wiki/Packaging/SourceURL ). In this case: # Desktop file, sent upstream for inclusion Source1: nested.desktop ** You are missing BuildRequires: desktop-file-utils ** I think you should use %F instead of %f in the desktop file; %F supports multiple files to be opened. Also, I think you should add the Utility; category. ** AFAIK it is standard Fedora practice to use %{_datadir}/man/man1/nested.1.* instead of %{_datadir}/man/man1/nested.1.gz since it is conceivable that the compression format of man pages might change in the future. But this is nitpicking. Please address the issues above before import to git, and send the updated desktop file upstream. This package has been APPROVED -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review