Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #10 from Michael Schwendt <mschwendt@xxxxxxxxx> --- > 'Patch1: rasmol-2.7.5-23Jul09-gtk.patch' becomes > 'Patch1: %{name}-2.7.5-23Jul09-gtk.patch' Not necessarily a good idea to do that. These tags refer to files with a hardcoded file name, because they have been checked into git with those file names. If %name changes, the file names don't change automatically. But if the patches still apply afterwards, there is no need to rename them. > - update-desktop-database is invoked when required But is it required? $ grep -i mime *.desktop $ http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database > Icon cache updated in rasmol package, no more icons for rasmol-gtk. What does that mean? Have you added the scriptlets in release -4? If so, please mention such details in the %changelog. It isn't easy to follow this review when you mention changes in bugzilla, which don't match the %changelog. The previous src.rpm is gone already, too, and as I don't have a copy of it, rpmdiff cannot be used. So, with regard to update-desktop-database and gtk-update-icon-cache I can only guess that there is some confusion. > Requires: %{name} = %{version}-%{release} https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > %package doc > Requires: %{name} = %{version}-%{release} Please don't add a base dep to -doc subpackages. Documentation packages ought to stay free of such dependencies. Imagine someone who wants to peruse the documentation without being forced to install the application (and possibly lots of dependencies). In case you worry about ownership of %{_datadir}/%{name}/, having multiple packages own the same dir is okay with current packaging guidelines. https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership > # 64b version require one more definition > %if "%{?_lib}" == "lib64" > %patch2 -p1 Dangerous. This assumes that every 64-bit platform uses lib64, even if it may not support multilib. > # Fix harcoded vars > sed -i -e 's|PKGDIR = $(HOME)|PKGDIR = /usr|' -e 's|/usr/local|/usr|g' > -e 's|RASMOLDIR = $(USRLIBDIR)/rasmol/|RASMOLDIR = %{_datadir}/rasmol/|' \ Without checking each of these substitutions, please consider adding guards (e.g. with a grep before or afterwards). Alternatively, add a comment that explains whether the %files section guards against failing substitutions already. Sed substitutions are fragile. If a match fails, because the target file has changed, no substitution would be performed. That's a case that may cause the build to fail later (e.g. due to the non-matching %files section, which would be good) or succeed with installing files in wrong locations or building the software with wrong built-in paths (which would be bad). -- 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=DtBPP90DvH&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review