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 at tummy.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|nobody at fedoraproject.org |kevin at tummy.com
Flag| |fedora-review?
------- Additional Comments From kevin at tummy.com 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.