Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: fedora-setup-keyboard - Hal keyboard layout callout
https://bugzilla.redhat.com/show_bug.cgi?id=486698
Summary: Review Request: fedora-setup-keyboard - Hal keyboard layout callout Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: adel.gadllah@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://193.200.113.196/apache2-default/rpm/fedora-setup-keyboard.spec SRPM URL: http://193.200.113.196/apache2-default/rpm/fedora-setup-keyboard-0.2-1.fc10.... Description: fedora-setup-keyboard gets invoked by hal to apply the keyboard layout defined in /etc/sysconfig/keyboard.
rpmlint output: fedora-setup-keyboard.x86_64: W: no-documentation 3 packages and 1 specfiles checked; 0 errors, 1 warnings.
(There are no docs)
koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1144341
NOTE: The files in this packages conflict with the ones shipped in xorg-x11-server-Xorg. This is intentional, it is going to replace them (X will than depend on this package).
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=486698
Adel Gadllah adel.gadllah@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |483817
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=486698
Peter Hutterer peter.hutterer@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |peter.hutterer@redhat.com
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=486698
Rudolf Kastl che666@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |che666@gmail.com
--- Comment #1 from Rudolf Kastl che666@gmail.com 2009-02-23 04:18:35 EDT --- why doesent it obsolete the package if it is going to replace it?
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=486698
--- Comment #2 from Adel Gadllah adel.gadllah@gmail.com 2009-02-23 08:14:09 EDT --- (In reply to comment #1)
why doesent it obsolete the package if it is going to replace it?
Because it doesn't. It does replace FILES shipped in the xserver package.
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=486698
Jens Petersen petersen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora-i18n-bugs@redhat.com | |, petersen@redhat.com
--- Comment #3 from Jens Petersen petersen@redhat.com 2009-02-24 18:58:49 EDT --- I think the package should probably "Requires: hal" for the %{_datadir}/hal/fdi/policy/10osvendor directory.
A license file is really required too.
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=486698
--- Comment #4 from Jens Petersen petersen@redhat.com 2009-02-24 19:01:10 EDT --- Otherwise rpmlint output is clean - a license file would also get rid of the no documentation warning. :)
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=486698
--- Comment #5 from Jens Petersen petersen@redhat.com 2009-02-24 19:05:07 EDT ---
NOTE: The files in this packages conflict with the ones shipped in xorg-x11-server-Xorg. This is intentional, it is going to replace them (X will than depend on this package).
It might be worth adding a "Conflicts: xorg-x11-server-Xorg < (last fedora-setup-keyboard.py version-(release+1))" for this.
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=486698
--- Comment #6 from Adel Gadllah adel.gadllah@gmail.com 2009-02-25 13:01:05 EDT --- (In reply to comment #5)
NOTE: The files in this packages conflict with the ones shipped in xorg-x11-server-Xorg. This is intentional, it is going to replace them (X will than depend on this package).
It might be worth adding a "Conflicts: xorg-x11-server-Xorg < (last fedora-setup-keyboard.py version-(release+1))" for this.
Well can't do this unless the xorg package is changed to use this fedora-setup-keyboard. (As xorg builds might be made during this review), besides it already conflicts with this packaged via file conflict.
Fixed the other 2 issues (license file and hal requirement)
http://193.200.113.196/apache2-default/rpm/fedora-setup-keyboard.spec http://193.200.113.196/apache2-default/rpm/fedora-setup-keyboard-0.3-1.fc10....
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=486698
--- Comment #7 from Peter Hutterer peter.hutterer@redhat.com 2009-02-25 17:53:19 EDT --- I'll add the conflicts line once the package is available and the xorg-x11-server package got its requires on it.
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=486698
Jens Petersen petersen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |petersen@redhat.com Flag| |fedora-review+
--- Comment #8 from Jens Petersen petersen@redhat.com 2009-03-01 19:23:12 EDT --- Apologies for the slow response - somehow missed your updates in my bugzilla folder last week.
Thanks for the update. Here is my review:
+:ok, !:needs fixing
MUST Items: [+] MUST: rpmlint output
rpmlint is now clean. :)
[*] MUST: Package Naming Guidelines [+] MUST: spec file name must match base package %{name} [+] MUST: Packaging Guidelines. [+] MUST: Licensing Guidelines [!] MUST: License field in the package spec file must match actual license.
The license is MIT not BSD!
[+] MUST: include license files in %doc if available in source [+] MUST: The spec file must be written in American English and be legible. [+] MUST: source md5sum matches upstream release
6bcb3d6a9f31eddd69aac5df3b50dd98 fedora-setup-keyboard-0.3.tar.bz2
[+] MUST: must successfully compile and build into binary rpms on one main arch [+] MUST: All build dependencies must be listed in BuildRequires [no shared libs] [+] MUST: A package must own all directories that it creates. [+] MUST: A package must not contain any duplicate files in the %files listing. [+] MUST: Permissions on files must be set properly. [+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: Each package must consistently use macros [+] MUST: The package must contain code, or permissable content. [+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [no devel files] [+] MUST: Packages must not own files or directories already owned by other packages. [+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [+] MUST: All filenames in rpm packages must be valid UTF-8.
SHOULD Items: [+] 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. [+] SHOULD: The reviewer should test that the package builds in mock.
http://koji.fedoraproject.org/koji/taskinfo?taskID=1212001
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
Please be sure to fix the License field before importing and the package is APPROVED for inclusion in Fedora.
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=486698
--- Comment #9 from Adel Gadllah adel.gadllah@gmail.com 2009-03-02 02:19:30 EDT ---
Please be sure to fix the License field before importing and the package is APPROVED for inclusion in Fedora.
Fixed in 0.3-2, thanks for the review.
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=486698
Adel Gadllah adel.gadllah@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #10 from Adel Gadllah adel.gadllah@gmail.com 2009-03-02 02:21:13 EDT --- New Package CVS Request ======================= Package Name: fedora-setup-keyboard Short Description: Hal keyboard layout callout Owners: drago01, whot Branches: F-10 InitialCC:
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=486698
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #11 from Kevin Fenzi kevin@tummy.com 2009-03-02 19:36:18 EDT --- cvs done.
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=486698
Jens Petersen petersen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
--- Comment #12 from Jens Petersen petersen@redhat.com 2009-03-03 22:45:02 EDT --- http://koji.fedoraproject.org/koji/buildinfo?buildID=92365
package-review@lists.fedoraproject.org