Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Merge Review: system-config-boot https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226453 kevin@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |kevin@xxxxxxxxx Flag| |fedora-review? ------- Additional Comments From kevin@xxxxxxxxx 2007-03-14 01:16 EST ------- OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches See below - License file included in package OK - Spec in American English OK - Spec is legible. See below - Sources match upstream md5sum: See below - Package needs ExcludeArch OK - BuildRequires correct OK - Spec handles locales/find_lang OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. See below - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. See below - Package is a GUI app and has a .desktop file OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. See below - Package doesn't own any directories other packages own. See below- Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. OK - Should have sane scriptlets. OK - Should have dist tag OK - Should package latest version 2 outstanding bugs - check for outstanding bugs on package. Issues: 1. Minor: might include a copy of the GPL. 2. Since redhat/fedora is upstream for this, can you make a note in the spec as suggested in: http://www.fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e 3. I assume the reason it only builds on ix86/x86_64 is that it only understands lilo/grub? Might be worth filing a bug and noting it in the spec and see if some ppc folk are interested in contributing yaboot support. 4. Please use one of the preferred buildroots, such as: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 5. Do not use %makeinstall. See: http://www.fedoraproject.org/wiki/PackagingDrafts/MakeInstall 6. The desktop file is missing a valid Main Category, see: http://standards.freedesktop.org/menu-spec/latest/apa.html Suggest: System or Settings be added. Without this, this tool shows up under a "Other" menu in Xfce. 7. The buildrequires are not all needed, suggest changing: BuildRequires: python >= 0:2.2, perl, gettext, glibc-devel, gcc, desktop-file-utils, yelp, perl-XML-Parser to BuildRequires: python, gettext, desktop-file-utils, yelp, perl-XML-Parser 8. Shouldn't the firstboot package own %dir /usr/share/firstboot/ %dir /usr/share/firstboot/modules and not this package? 9. 2 outstanding bugs. Might look if either can be resolved easily. 10. rpmlint says: a) E: system-config-boot no-binary I assume this is not noarch since it can be only run on ix86/x86_64? b) W: system-config-boot conffile-without-noreplace-flag /etc/pam.d/system-config-boot W: system-config-boot conffile-without-noreplace-flag /etc/security/console.apps/system-config-boot Suggest: should those be (noreplace)? c) W: system-config-boot no-documentation No docs available? d) E: system-config-boot non-executable-script /usr/share/firstboot/modules/boot_gui.py 0644 E: system-config-boot non-executable-script /usr/share/system-config-boot/system-config-boot.py 0644 Suggest: remove the #!/usr/bin/python from those. e) W: system-config-boot unversioned-explicit-obsoletes redhat-config-boot Suggest: add a version here? or just remove it at this point? f) W: system-config-boot rpm-buildroot-usage %prep rm -rf $RPM_BUILD_ROOT E: system-config-boot no-cleaning-of-buildroot %install Suggest: Move the rm from prep to the top of install? g) W: system-config-boot macro-in-%changelog dist W: system-config-boot macro-in-%changelog dist Suggest: Change occurances of %dist in the changelog with %%dist h) E: system-config-boot-debuginfo empty-debuginfo-package I guess you need to add %define debug_package %{nil} if this really has to be an arch package. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review