https://bugzilla.redhat.com/show_bug.cgi?id=1019437 Lubomir Rintel <lkundrak@xxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW --- Comment #7 from Lubomir Rintel <lkundrak@xxxxx> --- (In reply to Mario Blättermann from comment #1) > # Bundles webp, which is BSD > License: GPLv2+ and BSD > > Libwebp is available separately for Fedora. As a consequence, you remove the > bundled stuff: > > # Make sure we don't build bundled stuff > rm -rf src/3rdparty/* > > In this case you don't need to recognize the license of libwebp. But > wouldn't it need libwebp-devel as a build requirement? It does not get built. I have clarified the comment. > rm -rf $RPM_BUILD_ROOT > %defattr(-,root,root,-) > This is default for ages and could be safely dropped. Done. > Your package contains translation files. That's why you need to use > %find_lang: > http://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files Done, thought it does not play well with some translations. Will report upstream. (In reply to Antonio Trande from comment #2) > > # Bundles webp, which is BSD > > License: GPLv2+ and BSD > > but you removed the Webp by > > > # Make sure we don't build bundled stuff > > rm -rf src/3rdparty/* > > Webp is installed as dependence. See above. > Please, remove %defattr(-,root,root,-) Done, see above. > [!]: Package consistently uses macros (instead of hard-coded directory > names). > > Please, use %{name} macro instead of 'eyesight' No. That serves no purpose, harms readability and is not required by the guidelines. > Please, remove rm -rf $RPM_BUILD_ROOT Done. Thank you for the review. Here's the updated package: SPEC: http://v3.sk/~lkundrak/SPECS/eyesight.spec SRPM: http://v3.sk/~lkundrak/SRPMS/eyesight-0.1.1-2.fc20.src.rpm -- 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