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=755510 --- Comment #4 from nicolas.vieville@xxxxxxxxxxxxxxxxxxxx 2011-12-03 13:30:39 EST --- (In reply to comment #3) > Initial review, although this requires an official sponsor, and I am not one. Thanks for the review. > I'm not sure how to handle you having multiple srpms. I've done this review on > the Fedora 16 srpm > I thought one srpm should be able to build for 15 and 16. Yes this should be done, but in this case sources are different and not compatible, so 2 srpm files were build with 2 different sources tarball files. Never mind, I've splitted the files in two independent sets of files: one for F-15 and one for F-16 > [ snip ] > > [FAIL] MUST: The sources used to build the package must match the upstream > source, as provided in the spec URL. Reviewers should use md5sum for this task. > If no upstream URL can be specified for this package, please see the Source URL > Guidelines for how to deal with this. > > $ wget > https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet/tarball/master/paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz > --2011-11-29 15:31:53-- > https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet/tarball/master/paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz > ...snip... > > 2011-11-29 15:31:53 (465 KB/s) - “master” saved [21050/21050] > > $ md5sum master > paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz > ce12f169445b20e3686b15a3e4c25d6b master # wget > ce12f169445b20e3686b15a3e4c25d6b > paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz > #extracted from srpm As sources tarball files can only be downloaded from github via web links redirected, wget fails probably in interpreting javascripts and then cannot rename the tarball file as the browser can do it. That's why you get a file named "master" instead of paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz. To deal with this and to try to be as closed as possible of Fedora packaging guides, I've modified the spec file and deleted the URL string and kept only the tarball file name. I also added comments in the spec file to explain how to download the right file. Hope this is the right manner to deal with such a case. > [ snip ] > > [?PASS?] MUST: The spec file MUST handle locales properly. This is done by > using > the %find_lang macro. Using %{_datadir}/locale/* is strictly forbidden. > > I see that the tranlations are installed into the forbidden directory but then > %find_lang is called. > I think this is correct but need a follow up on this PASS I have to install locales in %{_datadir}/locale/$LANG for each translation before calling %find_lang, because there's no Makefile with install section in it for this Gnome shell extension. I've tried several alternatives but none of them were satisfying and causes %find_lang to fail. Every suggestion on this point are welcome. > [ snip ] > > [FAIL] MUST: Packages containing GUI applications must include a > %{name}.desktop > file, and that file must be properly installed with desktop-file-install in the > %install section. If you feel that your packaged GUI application does not need > a .desktop file, you must put a comment in the spec file with your explanation. > > I see the .desktop file but not the desktop-file-install in the %install > section Corrected: desktop-file-install added in the %install section > [ snip ] > > [FAIL] SHOULD: The description and summary sections in the package spec file > should contain translations for supported Non-English languages, if available. Sorry for that, but the upstream sources lacks of it, and my skill in such things isn't sufficient to produce these translations. Any suggestions for that are welcome. > [ snip ] > > [FAIL] SHOULD: your package should contain man pages for binaries/scripts. If > it doesn't, work with upstream to add them where they make sense. Upstream developers only provides the README.md file and didn't felt necessary to build a man page for this Gnome shell extension. I hope this explanation would be sufficient and I'm open to any suggestion to modify the point that must be corrected. Here are the new URLs for SPEC files, the new URLs for SRPMS files and the new rpmlint logs. F-16 Spec URL: http://dl.dropbox.com/u/25699833/Fedora/F-16/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet.spec F-16 SRPM URL: http://dl.dropbox.com/u/25699833/Fedora/F-16/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-1.99-1.fc16.src.rpm F-15 Spec URL: http://dl.dropbox.com/u/25699833/Fedora/F-15/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet.spec F-15 SRPM URL: http://dl.dropbox.com/u/25699833/Fedora/F-15/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-1.99-1.fc15.src.rpm Project URL: https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet Rpmlint for F-16 $ rpmlint gnome-shell-extension-system-monitor-applet.spec gnome-shell-extension-system-monitor-applet.spec: W: invalid-url Source0: paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz 0 packages and 1 specfiles checked; 0 errors, 1 warnings. $ rpmlint gnome-shell-extension-system-monitor-applet-1.99-1.fc16.src.rpm gnome-shell-extension-system-monitor-applet.src: W: invalid-url Source0: paradoxxxzero-gnome-shell-system-monitor-applet-1.99-83-gaffc741.tar.gz 1 packages and 0 specfiles checked; 0 errors, 1 warnings. $ rpmlint gnome-shell-extension-system-monitor-applet-1.99-1.fc16.noarch.rpm gnome-shell-extension-system-monitor-applet.noarch: W: devel-file-in-non-devel-package /usr/bin/system-monitor-applet-config gnome-shell-extension-system-monitor-applet.noarch: W: no-manual-page-for-binary system-monitor-applet-config 1 packages and 0 specfiles checked; 0 errors, 2 warnings. Rpmlint for F-15 $ rpmlint gnome-shell-extension-system-monitor-applet.specgnome-shell-extension-system-monitor-applet.spec: E: specfile-error error: Fedora 15 needed. Please examine the spec file. gnome-shell-extension-system-monitor-applet.spec: E: specfile-error error: line 3: Unknown tag: exit 1 gnome-shell-extension-system-monitor-applet.spec: E: specfile-error error: query of specfile gnome-shell-extension-system-monitor-applet.spec failed, can't parse 0 packages and 1 specfiles checked; 3 errors, 0 warnings. $ rpmlint gnome-shell-extension-system-monitor-applet-1.99-1.fc15.src.rpm gnome-shell-extension-system-monitor-applet.src: E: specfile-error error: Fedora 15 needed. Please examine the spec file. gnome-shell-extension-system-monitor-applet.src: E: specfile-error error: line 3: Unknown tag: exit 1 gnome-shell-extension-system-monitor-applet.src: E: specfile-error error: query of specfile /tmp/rpmlint.gnome-shell-extension-system-monitor-applet-1.99-1.fc15.src.rpm.cdM6aC/gnome-shell-extension-system-monitor-applet.spec failed, can't parse 1 packages and 0 specfiles checked; 3 errors, 0 warnings. $ rpmlint /var/lib/mock/fedora-15-x86_64/result/gnome-shell-extension-system-monitor-applet-1.99-1.fc15.noarch.rpm gnome-shell-extension-system-monitor-applet.noarch: W: devel-file-in-non-devel-package /usr/bin/system-monitor-applet-config gnome-shell-extension-system-monitor-applet.noarch: W: no-manual-page-for-binary system-monitor-applet-config 1 packages and 0 specfiles checked; 0 errors, 2 warnings. Thanks for reviewing. Cordially, -- NVieville -- 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