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-display https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226456 kevin@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@xxxxxxxxxxxxxxxxx |kevin@xxxxxxxxx Flag| |fedora-review? ------- Additional Comments From kevin@xxxxxxxxx 2007-03-20 23:59 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: OK - Package needs ExcludeArch - 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 has rm -rf RPM_BUILD_ROOT at top of %install OK - 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. OK - Package doesn't own any directories other packages own. OK - 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 have dist tag OK - Should package latest version 73 bugs outstanding - check for outstanding bugs on package. Issues: 1. The URL here seems to point to a 404. Is there some better page to point the URL to? 2. Since redhat/fedora is upstream for this package, can you add a note as suggested in: http://www.fedoraproject.org/wiki/Packaging/SourceURL#head-413e1c297803cfa9de0cc4c56f3ac384bff5dc9e 3. Please use one of the preferred buildroots, such as: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 4. Why is this preun needed? %preun if [ -d /usr/share/system-config-display ] ; then rm -rf /usr/share/system-config-display/*.pyc fi 5. Some of the translation files say: po/lt.po:# This file is distributed under the same license as the PACKAGE package. Would be nice to say "system-config-display" there instead of PACKAGE? 6. Should add a rm -rf $RPM_BUILD_ROOT to the top of the %install section. 7. 73 outstanding bugs. Perhaps it would be good to do some triage on those and see if anything can easily be addressed as part of this review cleanup? Or, given the number perhaps we could get a group together to assist you at some point? Particular to packaging: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=216471 and https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=212911 should be fixed by removing the preun (point 4) 8. rpmlint says: a) E: system-config-display obsolete-not-provided redhat-config-xfree86 E: system-config-display obsolete-not-provided Xconfigurator W: system-config-display unversioned-explicit-obsoletes redhat-config-xfree86 W: system-config-display unversioned-explicit-obsoletes Xconfigurator Are these old Obsoletes needed anymore? b) W: system-config-display conffile-without-noreplace-flag /etc/pam.d/system-config-display W: system-config-display conffile-without-noreplace-flag /etc/security/console.apps/system-config-display Should those be noreplace? c) W: system-config-display no-documentation Ignore d) E: system-config-display script-without-shebang /usr/share/system-config-display/xConfigDialog.py E: system-config-display script-without-shebang /usr/share/system-config-display/display.glade E: system-config-display script-without-shebang /usr/share/system-config-display/screenSizePreview.py E: system-config-display script-without-shebang /usr/share/system-config-display/monitorDialog.py E: system-config-display script-without-shebang /usr/share/system-config-display/videocardDialog.py All those should be mode 644? e) W: system-config-display dangerous-command-in-%preun rm Remove the preun? f) W: system-config-display prereq-use gtk2 >= 2.6 Is that prereq needed? Can it be a Requires instead? g) W: system-config-display prereq-use hicolor-icon-theme Should this be "Requires(postun)" and "Requires(post)" h) E: system-config-display no-cleaning-of-buildroot %install See point 6. 9. Is the "Requires: metacity" really needed? why? -- 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