Bug 546686

Updated to version 1.1.0.1

Spec URL: http://sagarun.fedorapeople.org/SPECS/cricscore-applet.spec
SRPM URL: http://sagarun.fedorapeople.org/SRPMS/cricscore-applet-1.1.0.2-1.fc12.src.rpm


koji builds: 

F12: http://koji.fedoraproject.org/koji/taskinfo?taskID=1966209
EPEL: http://koji.fedoraproject.org/koji/taskinfo?taskID=1966226

Any cricket lovers here :-) ? Any one willing to review/package this package?

quick review:

- Please justify the explicit Requires
http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

- The src tar contains a Makefile, why not use use make install with required directories as arguments instead of explicitly installing the files yourself?

make install DESTDIR=%{buildroot} etc?

minor changes:

summary could use an "A" : A cricket score applet for GNOME

description could be changed to : "%{name} gets cricket scores to your panel", instead of "The goal of.. "

regards,
Ankur

Ankur, Thanks for the review.

(In reply to comment #2)
> quick review:
> 
> - Please justify the explicit Requires
> http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires
> 

Yes, those requires are not needed, i removed them from spec file. http://koji.fedoraproject.org/koji/taskinfo?taskID=2122121


> - The src tar contains a Makefile, why not use use make install with required
> directories as arguments instead of explicitly installing the files yourself?
> 
> make install DESTDIR=%{buildroot} etc?

Done.

> 
> minor changes:
> 
> summary could use an "A" : A cricket score applet for GNOME
> 
> description could be changed to : "%{name} gets cricket scores to your panel",
> instead of "The goal of.. "

Done.


SPEC URL: http://sagarun.fedorapeople.org/SPECS/cricscore-applet.spec
SRPM URL: http://sagarun.fedorapeople.org/SRPMS/cricscore-applet-1.1.0.2-2.fc12.src.rpm

complete review:

+ = OK
- = NA
? = issue


+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.

+ Sources match upstream md5sum:
[Ankur1@localhost rpmbuild]$ md5sum cricscore-1.1.0.2.tar.gz 
fcd9a577640e1f25c45a24b40f93e476  cricscore-1.1.0.2.tar.gz
[Ankur1@localhost rpmbuild]$ md5sum SOURCES/cricscore-1.1.0.2.tar.gz 
fcd9a577640e1f25c45a24b40f93e476  SOURCES/cricscore-1.1.0.2.tar.gz


- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
+ Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
+ Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

? Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
http://koji.fedoraproject.org/koji/taskinfo?taskID=2122121

+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
? No rpmlint output.

- final provides and requires are sane:
(include output of for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done
manually indented after checking each line.  I also remove the rpmlib junk and anything provided by glibc.)

SHOULD Items:

+ Should build in mock.
+ Should build on all supported archs
+ Should function as described.
+ Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)


XXXX Issues XXXX

1. use parallel make?
https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make

2. since its a GUI, I think a desktop file is needed. 
https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

Please make a desktop file as SOURCE1 and also submit this upstream?

3. rpmlint gives some errors:
[Ankur1@localhost SOURCES]$ rpmlint  ~/Downloads/cricscore-applet-1.1.0.2-2.fc12.noarch.rpm 
cricscore-applet.noarch: E: script-without-shebang /usr/libexec/cricscore-applet/cricscore_prefs.py
cricscore-applet.noarch: E: script-without-shebang /usr/libexec/cricscore-applet/gnome_cricscore_globals.py
cricscore-applet.noarch: E: script-without-shebang /usr/lib/bonobo/servers/cricketscore.server
cricscore-applet.noarch: E: script-without-shebang /usr/share/cricscore-applet/GNOME_CricScoreApplet.xml
1 packages and 0 specfiles checked; 4 errors, 0 warnings.

trivial fix:
http://fedoraproject.org/wiki/Packaging_tricks#Add_shebang


These are all. Once these are fixed, you're good to go ! :)

regards,
Ankur

(I
> - final provides and requires are sane:
> (include output of for i in *rpm; do echo $i; rpm -qp --provides $i; echo =;
> rpm -qp --requires $i; echo; done
> manually indented after checking each line.  I also remove the rpmlib junk and
> anything provided by glibc.)
> 

[Package@localhost rpmbuild]$ for i in *rpm; do echo $i; rpm -qp --provides $i; echo =;
> rpm -qp --requires $i; echo; done
cricscore-applet-1.1.0.2-2.fc12.noarch.rpm
cricscore-applet = 1.1.0.2-2.fc12
=
/usr/bin/env  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

cricscore-applet-1.1.0.2-2.fc12.src.rpm
=
pygtk2-devel  
gettext  
python-devel >= 2.4
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1

(In reply to comment #4)

> XXXX Issues XXXX
> 
> 1. use parallel make?
> https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
> 


Will make use of the macro %{_smp_mflags}

> 2. since its a GUI, I think a desktop file is needed. 
> https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
> 
> Please make a desktop file as SOURCE1 and also submit this upstream?
> 

Not needed? I checked another panel applet named deskbar, it does not have any .desktop files. Do i still need to add a .desktop file?


> 3. rpmlint gives some errors:
> [Ankur1@localhost SOURCES]$ rpmlint 
> ~/Downloads/cricscore-applet-1.1.0.2-2.fc12.noarch.rpm 
> cricscore-applet.noarch: E: script-without-shebang
> /usr/libexec/cricscore-applet/cricscore_prefs.py
> cricscore-applet.noarch: E: script-without-shebang
> /usr/libexec/cricscore-applet/gnome_cricscore_globals.py
> cricscore-applet.noarch: E: script-without-shebang
> /usr/lib/bonobo/servers/cricketscore.server
> cricscore-applet.noarch: E: script-without-shebang
> /usr/share/cricscore-applet/GNOME_CricScoreApplet.xml
> 1 packages and 0 specfiles checked; 4 errors, 0 warnings.
> 
> trivial fix:
> http://fedoraproject.org/wiki/Packaging_tricks#Add_shebang
> 

Will fix.

(In reply to comment #6)
> (In reply to comment #4)
> 
> > XXXX Issues XXXX
> > 
> > 1. use parallel make?
> > https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
> > 
> 
> 
> Will make use of the macro %{_smp_mflags}
> 
> > 2. since its a GUI, I think a desktop file is needed. 
> > https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
> > 
> > Please make a desktop file as SOURCE1 and also submit this upstream?
> > 
> 
> Not needed? I checked another panel applet named deskbar, it does not have any
> .desktop files. Do i still need to add a .desktop file?
> 

yeah, checked, not needed. 

> 
> > 3. rpmlint gives some errors:
> > [Ankur1@localhost SOURCES]$ rpmlint 
> > ~/Downloads/cricscore-applet-1.1.0.2-2.fc12.noarch.rpm 
> > cricscore-applet.noarch: E: script-without-shebang
> > /usr/libexec/cricscore-applet/cricscore_prefs.py
> > cricscore-applet.noarch: E: script-without-shebang
> > /usr/libexec/cricscore-applet/gnome_cricscore_globals.py
> > cricscore-applet.noarch: E: script-without-shebang
> > /usr/lib/bonobo/servers/cricketscore.server
> > cricscore-applet.noarch: E: script-without-shebang
> > /usr/share/cricscore-applet/GNOME_CricScoreApplet.xml
> > 1 packages and 0 specfiles checked; 4 errors, 0 warnings.
> > 
> > trivial fix:
> > http://fedoraproject.org/wiki/Packaging_tricks#Add_shebang
> > 
> 
> Will fix.    

please upload the final srpm, and we can finish up this review :)

Ankur

Spec URL: http://sagarun.fedorapeople.org/SPECS/cricscore-applet.spec
SRPM URL:
http://sagarun.fedorapeople.org/SRPMS/cricscore-applet-1.1.0.2-2.fc12.src.rpm


Note: rpmlint on results in following warnings

bash-4.0$ rpmlint ~/rpmbuild/SRPMS/cricscore-applet-1.1.0.2-2.fc12.src.rpm 
cricscore-applet.src:48: W: libdir-macro-in-noarch-package (main package)
%{_libdir}/bonobo/servers/cricketscore.server
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
bash-4.0$ 

This warning can be ignored IMO. what is your take on this issue?

(In reply to comment #8)
> Spec URL: http://sagarun.fedorapeople.org/SPECS/cricscore-applet.spec
> SRPM URL:
> http://sagarun.fedorapeople.org/SRPMS/cricscore-applet-1.1.0.2-2.fc12.src.rpm
> 
> 
> Note: rpmlint on results in following warnings
> 
> bash-4.0$ rpmlint ~/rpmbuild/SRPMS/cricscore-applet-1.1.0.2-2.fc12.src.rpm 
> cricscore-applet.src:48: W: libdir-macro-in-noarch-package (main package)
> %{_libdir}/bonobo/servers/cricketscore.server
> 1 packages and 0 specfiles checked; 0 errors, 1 warnings.
> bash-4.0$ 
> 
> This warning can be ignored IMO. what is your take on this issue?    

hey,

I asked some folks on -devel about this. The only solution is to remove the noarch tag and build separate packages for the different arches. That should fix it. 

Ankur

(In reply to comment #9)


> I asked some folks on -devel about this.

Thanks a lot ankur.


> The only solution is to remove the noarch tag and build separate packages for >the different arches. That should fix it. 

I removed the noarch tag and rebuild the RPM, Unfortunately rpmlint on rpm shows "E: No binary" 

https://fedoraproject.org/wiki/Common_Rpmlint_issues#no-binary

(In reply to comment #10)
> (In reply to comment #9)
> 
> 
> > I asked some folks on -devel about this.
> 
> Thanks a lot ankur.
> 
> 
> > The only solution is to remove the noarch tag and build separate packages for >the different arches. That should fix it. 
> 
> I removed the noarch tag and rebuild the RPM, Unfortunately rpmlint on rpm
> shows "E: No binary" 
> 
> https://fedoraproject.org/wiki/Common_Rpmlint_issues#no-binary    

hey,

This can be ignored. The package is supposed to be a noarch. However, the inclusion of the bonobo stuff in libdir needs the noarch to go. 

Please upload the latest spec and src rpm.

Ankur

Updated to latest version.

Spec URL: http://sagarun.fedorapeople.org/SPECS/cricscore-applet.spec
SRPM URL: http://sagarun.fedorapeople.org/SRPMS/cricscore-applet-1.1.0.3-1.fc12.src.rpm

hey,

Looks good. 


XXXXX APPROVED XXXXX

New Package CVS Request
=======================
Package Name: cricscore-applet
Short Description: Cricket Score applet for gnome panel
Owners: sagarun
Branches: F12 F13

CVS done (by process-cvs-requests.py).