https://bugzilla.redhat.com/show_bug.cgi?id=755510 --- Comment #35 from nicolas.vieville@xxxxxxxxxxxxxxxxxxxx --- (In reply to comment #31) Jeff, Thank you very much for your review and your advice. This make this package more conform regarding Fedora Packaging Guidelines. Sorry for my late response, reasons are in comment #33 beginning. > Will follow up with the source github url in another comment. It does appear > that I was wrong about find -exec simplifying the translations, so do as you > please. You were right about find -exec, the translations are now in line with the "oneliner" directive and probably are much stronger in catching errors, although a little bit difficult to read. I have kept them. > Package Review > ============== > > Key: > - = N/A > x = Pass > ! = Fail > ? = Not evaluated > > > > ==== Generic ==== > [x]: EXTRA Rpmlint is run on all installed packages. > Note: There are rpmlint messages (see attachment). > [x]: EXTRA Spec file according to URL is the same as in SRPM. > [X]: MUST Package is licensed with an open-source compatible license and > meets > other legal requirements as defined in the legal section of Packaging > Guidelines. > [x]: MUST Package successfully compiles and builds into binary rpms on at > least one supported primary architecture. > [-]: MUST %build honors applicable compiler flags or justifies otherwise. > [x]: MUST All build dependencies are listed in BuildRequires, except for any > that are listed in the exceptions section of Packaging Guidelines. > [x]: MUST Buildroot is not present > Note: Unless packager wants to package for EPEL5 this is fine > [?]: MUST Changelog in prescribed format. NOTE: the versioning may change > based on github url changes. I agree. Do you think that git commit numbers should be replaced by date? About version number, I try to follow upstream numbers and Packaging Guidelines. Any advice on resolving this point would be appreciated. > [X]: MUST Package contains no bundled libraries. > [x]: MUST Package has no %clean section with rm -rf %{buildroot} (or > $RPM_BUILD_ROOT) > Note: Clean would be needed if support for EPEL is required > [X]: MUST Sources contain only permissible code or content. > [x]: MUST Each %files section contains %defattr if rpm < 4.4 > Note: Note: defattr macros not found. They would be needed for EPEL5 > [X]: MUST Package requires other packages for directories it uses. > [X]: MUST Package uses nothing in %doc for runtime. > [x]: MUST Permissions on files are set properly. > [x]: MUST Package does not contain duplicates in %files. > [x]: MUST Spec file lacks Packager, Vendor, PreReq tags. > [-]: MUST Large documentation files are in a -doc subpackage, if required. > [-]: MUST If (and only if) the source package includes the text of the > license(s) in its own file, then that file, containing the text of the > license(s) for the package is included in %doc. > [-]: MUST The spec file handles locales properly. > [X]: MUST Package consistently uses macro is (instead of hard-coded directory > names). > [x]: MUST Package is named using only allowed ascii characters. > [X]: MUST Package is named according to the Package Naming Guidelines. > [X]: MUST Package does not generate any conflict. > Note: Package contains no Conflicts: tag(s) > [X]: MUST Package obeys FHS, except libexecdir and /usr/target. > [X]: MUST Package must own all directories that it creates. > [X]: MUST Package does not own files or directories owned by other packages. > [?]: MUST Package installs properly. (trusting you here) I've followed previous Review Request for this piece of software, and the Fedora existing gnome-shell extensions method to get where and how to install this package components. As gnome-shell extensions actually can be installed in users account tree, but this one is globally installed like Fedora provided extensions, I hope this is the good way. > [X]: MUST Package is not relocatable. > [!]: MUST Requires correct, justified where necessary. NOTE: is > python3-gobject necessary? I have the extension installed on my system and > that package is not installed. Sorry, I missed it. It was necessary with python for settings in the previous version of this applet (F-15 and F-16). Fixed. I've also removed desktop-file-utils in BuildRequires section for the same reason. > [x]: MUST Rpmlint is run on all rpms the build produces. > Note: There are rpmlint messages (see attachment). > [x]: MUST Sources used to build the package match the upstream source, as > provided in the spec URL. > [X]: MUST Spec file is legible and written in American English. > [x]: MUST Spec file name must match the spec package %{name}, in the format > %{name}.spec. > [-]: MUST Package contains systemd file(s) if in need. > [x]: MUST File names are valid UTF-8. > [x]: SHOULD Reviewer should test that the package builds in mock. > [?]: SHOULD If the source package does not include license text(s) as a > separate file from upstream, the packager SHOULD query upstream to > include it. NOTE: license in README.md. Requested upstream today. Hope it will be accepted! https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet/issues/145 > [x]: SHOULD Dist tag is present. > [x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin, > /usr/sbin. > [-]: SHOULD Package functions as described. > [X]: SHOULD Latest version is packaged. > [X]: SHOULD Package does not include license text files separate from > upstream. > [X]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise > justified. > [!]: SHOULD SourceX / PatchY prefixed with %{name}. > Note: Patch0 (paradoxxxzero-gnome-shell-system-monitor-applet_fix_gnome- > shell_version_required.patch) Patch1 (paradoxxxzero-gnome-shell-system- > monitor-applet_fix_gettext_domain.patch) Source0 (paradoxxxzero-gnome- > shell-system-monitor-applet-2.0b1-123-g3117df5.tar.gz) One more thing I had to integrate in the packaging process. Thanks for catching it. Fixed > [x]: SHOULD SourceX is a working URL. > [-]: SHOULD Description and summary sections in the package spec file > contains > translations for supported Non-English languages, if available. > [?]: SHOULD Package should compile and build into binary rpms on all > supported > architectures. It should be as this package is made of javascript and depends on gnome-shell. I would appreciate if someone could show me how to meet this request and to verify it if necessary. > [?]: SHOULD %check is present and all tests pass. As there is no upstream test, what do you think if this point should be N/A? > [?]: SHOULD Packages should try to preserve timestamps of original installed > files. Any advice to achieve this would be appreciate if necessary. > [x]: SHOULD Spec use %global instead of %define. > > Issues: see [!] and NOTES. Here are the new SPEC/SRPMS/RPMS files modified to conform your Review. Thanks to your comment #32, rpmlint warnings disappeared. I wasn't aware that such a method was possible to download some sources tarball from github. Very good advice. As usual, older SPEC/RPMS/SRPMS files are no longer available on my Dropbox shared folder (F-15 and F-16 ones are still there). Rawhide Spec URL: http://dl.dropbox.com/u/25699833/Fedora/Rawhide/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet.spec Rawhide SRPM URL: http://dl.dropbox.com/u/25699833/Fedora/Rawhide/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git21f7cce.fc18.src.rpm Rawhide RPM URL: http://dl.dropbox.com/u/25699833/Fedora/Rawhide/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git21f7cce.fc18.noarch.rpm F-17 RPM URL: http://dl.dropbox.com/u/25699833/Fedora/F-17/gnome-shell-extension-system-monitor-applet/gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git21f7cce.fc17.noarch.rpm Project URL: https://github.com/paradoxxxzero/gnome-shell-system-monitor-applet rpmlint for Rawhide rpmlint gnome-shell-extension-system-monitor-applet.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint gnome-shell-extension-system-monitor-applet-2.0b1-0.1.git21f7cce.fc18.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Thank you again Jeff! Cordially, -- NVieville -- 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