Fedora Merge Review: gok http://cvs.fedora.redhat.com/viewcvs/devel/gok/ Initial Owner: davidz
MUST Items: xx - rpmlint is unclean on RPM + [rishi@freebook devel]$ rpmlint gok gok.x86_64: W: file-not-utf8 /usr/share/doc/gok-1.3.7/NEWS gok.x86_64: W: non-conffile-in-etc /etc/gconf/schemas/gok.schemas gok.x86_64: W: non-standard-group Desktop/Accessibility 1 packages and 0 specfiles checked; 0 errors, 3 warnings. [rishi@freebook devel]$ + rpmlint is unclean on SRPM [rishi@freebook SPECS]$ rpmlint /devel/rpmbuild/SRPMS/gok-2.25.2-1.fc9.src.rpm gok.src:251: W: macro-in-%changelog _lib gok.src: W: mixed-use-of-spaces-and-tabs (spaces: line 14, tab: line 19) gok.src: W: non-standard-group Desktop/Accessibility 1 packages and 0 specfiles checked; 0 errors, 3 warnings. [rishi@freebook SPECS]$ OK - follows Naming Guidelines OK - spec file is named as %{name}.spec xx - package does not meet Packaging Guidelines + Please consider fixing the mixed use of tabs and spaces in the BuildRequires and Requires tags. + Even though Fedora does not use the Group tag, please consider using one from /usr/share/doc/rpm-4.4.2.3/GROUPS. "User Interface/Desktops" is a probable choice. Orca also uses it. + 'Requires: htmlview' is needed because of Patch0. + Can gok-1.2.1-fixlogin.patch be removed from CVS? + Please convert /usr/share/doc/gok-1.3.7/NEWS from ISO-8859 to UTF-8. See: https://fedoraproject.org/wiki/PackageMaintainers/Common_Rpmlint_Issues#file-not-utf8 + According to https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make you should use 'make %{?_smp_mflags}' whenever possible. In this case since upstream supports parallel builds you should use it. + To preserve timestamps you could consider using: make install INSTALL="%{__install} -p" DESTDIR=$RPM_BUILD_ROOT + 'rm -rf $RPM_BUILD_ROOT/var/scrollkeeper' is not needed on recent distributions. + Replace %{_lib} with %%{_lib} in the %changelog. OK - license meets Licensing Guidelines OK - license field meets actual license + gok/keysym2ucs.c is in the public domain. OK - upstream license file included in %doc OK - spec file uses American English OK - spec file is legible OK - sources match upstream sources xx - package does not build successfully + Fails to build due to missing BuildRequires: http://koji.fedoraproject.org/koji/taskinfo?taskID=1023579 OK - ExcludeArch not needed xx - build dependencies incorrectly listed + 'BuildRequires: esound-devel' is missing. + 'BuildRequires: m4 perl(XML::Parser)' is redundant. OK - locales handled properly OK - no shared libraries OK - package is not relocatable xx - file and directory ownership + Since an icon has been installed under %{_datadir}/icons/hicolor there should be a runtime dependency on hicolor-icon-theme. However, is this icon really needed as there is no .desktop file? + The -devel sub-package should have 'Requires: gtk-doc' as it needs /usr/share/gtk-doc. OK - no duplicates in %file OK - file permissions set properly OK - %clean present OK - macros used consistently + Both gok and %{name} used. OK - contains code and permissable content OK - -doc is not needed OK - contents of %doc does not affect the runtime OK - no header files OK - no static libraries OK - -devel has *.pc file and requires pkgconfig OK - no library files OK - -devel requires base package OK - no libtool archives OK - %{name}.desktop file not provided + Changelog contains rationale for excluding it. OK - does not own files or directories owned by other packages OK - buildroot correctly prepped OK - all file names valid UTF-8 SHOULD Items: OK - upstream provides license text xx - no translations for description and summary xx - package fails to build in mock + 'BuildRequires: esound-devel' is missing. OK - package builds on all supported architectures OK - package functions as expected xx - scriptlets are not sane + There does not seem to be any .omf files provided by the package, yet the scrollkeeper scriptlets are being used. Why? It looks like gok-1.3.7-3.fc9.x86_64 had them. OK - subpackages other than -devel are not needed OK - pkgconfig files in -devel OK - no file dependencies
(In reply to comment #1) > xx - file and directory ownership > + Since an icon has been installed under %{_datadir}/icons/hicolor there > should be a runtime dependency on hicolor-icon-theme. However, is this > icon really needed as there is no .desktop file? Ofcourse it is needed for the icons in the window titles. So please add the dependency on hicolor-icon-theme.
Ping?
I've fixed most of the fixworthy things you listed. + Since an icon has been installed under %{_datadir}/icons/hicolor there should be a runtime dependency on hicolor-icon-theme. gok -> gtk2 -> hicolor-icon-theme + There does not seem to be any .omf files provided by the package, yet the scrollkeeper scriptlets are being used. W [mclasen@matthiasc devel]$ rpm -ql gok | grep omf /usr/share/omf/gok /usr/share/omf/gok/gok-C.omf
> + According to > https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make you should > use 'make %{?_smp_mflags}' whenever possible. In this case since upstream > supports parallel builds you should use it. You have missed this one. Sorry for not mentioning this earlier, but thanks to the new RPM in Rawhide the following Requires in the -devel sub-package are redundant: Requires: libgnomeui-devel Requires: atk-devel Requires: libbonobo-devel Requires: gtk2-devel Requires: gail-devel Requires: libwnck-devel Requires: esound-devel Requires: at-spi-devel Requires: pkgconfig [rishi@freebook tmp]$ rpm --requires -qp gok-devel-2.25.3-3.fc11.x86_64.rpm /usr/bin/pkg-config [...] pkgconfig(atk) pkgconfig(cspi-1.0) pkgconfig(esound) pkgconfig(gail) pkgconfig(gtk+-2.0) pkgconfig(libbonobo-2.0) pkgconfig(libgnomeui-2.0) pkgconfig(libspi-1.0) pkgconfig(libwnck-1.0) Even though https://fedoraproject.org/wiki/Packaging:Guidelines#Pkgconfig_Files still mandates the use of 'Requires: pkgconfig', Spot confirmed that they are not needed from Fedora 11 onwards.
> You have missed this one. No, I haven't. I didn't feel it was worth it. There's only so much time I can justify spending on irrelevant details of a spec file that make no difference whatsoever to the resulting package.
(In reply to comment #6) >> You have missed this one. > No, I haven't. I didn't feel it was worth it. There's only so much time I can > justify spending on irrelevant details of a spec file that make no difference > whatsoever to the resulting package. The guidelines recommend the use of parallel builds and there is a rationale behind it: http://www.redhat.com/archives/fedora-packaging/2007-July/msg00004.html Can I make the change on your behalf?
Thats not a rationale. Thats just spot saying "it should work". Do you know that it works in this case ?
(In reply to comment #8) > Do you know that it works in this case ? http://koji.fedoraproject.org/koji/taskinfo?taskID=1090498
So, if it makes you happy, commit it. Obsessing over trivia like this is what makes merge reviews so resented...
Merge-reviews are always topic of debate on fedora lists. If we have approved spec template as given in /etc/rpmdevtools/spectemplate-minimal.spec then why not megre-review packages simply follow them? Merge-review packages should not be considered a special case and exempted from what FESCo approved Packaging Guidelines say.
No need to jump on this.
I added '%{?_smp_mflags}' and took the liberty to remove the mixed tabs and spaces warnings from RPMLint. * Fri Jan 30 2009 Debarshi Ray <rishi> - 2.25.3-4 - Removed mixed use of tabs and spaces. - Use parallel make. +---------------------------------+ | This package is APPROVED by me. | +---------------------------------+ Please close this review as NEXTRELEASE.