Bug 235815

Nice job, I was hoping someone would tackle this caml stuff, and here you are.

I'll try todo some reviews on this, starting with the caml deps. If someone
wants to beat me to it, thats fine. As long as I haven't assigned the bug to me,
I haven't actually started reviewing.

Initial list with some things to fix after a quick initial look.

- The main "%files" is missing a %defattr, this should be added as the first
line after %files

- %files-data can be written mich shorter, it can (and should be) as simple as:

%files data
%defattr(-,root,root,-)
%doc COPYING CHANGES.txt AUTHORS
%{_datadir}/%{name}

  Also notice that %defattr is the first line after %files data, putting 
  %defattr at the end has no effects as %defattr is only applied to files listed
  below it, but read on...

- The data subpackage should require the main package, so that it gets removed 
  with it, but read on...

- The data package doesn't need to double some of the docs from the main package
  as it requires it, but read on...

- However since the data is in the same upstream tarbal as the sources, and
  this cannot be in a seperate srpm there is no use (what so ever) in having
  it in a subpackage, so just completely remove the data sub-package,
  and add the single line "%{_datadir}/%{name}" to the main %files list

- Please add an icon, just take any suitable graphics included and resize / edit
  it as nescesarry then install it under /usr/share/icons/hicolor/<size>/apps
  and add the necessary scripts to update the icon-cache. (See scriptlets wiki
  page)


p.s. Do you still need a sponsor? Once this and the deps have been reviewed I
can sponsor you if needed.

(In reply to comment #2)
> Initial list with some things to fix after a quick initial look.
> 
> - The main "%files" is missing a %defattr, this should be added as the first
> line after %files
> 
> - %files-data can be written mich shorter, it can (and should be) as simple as:
> 
> %files data
> %defattr(-,root,root,-)
> %doc COPYING CHANGES.txt AUTHORS
> %{_datadir}/%{name}
> 
>   Also notice that %defattr is the first line after %files data, putting 
>   %defattr at the end has no effects as %defattr is only applied to files listed
>   below it, but read on...
Okay shall do...
> - However since the data is in the same upstream tarbal as the sources, and
>   this cannot be in a seperate srpm there is no use (what so ever) in having
>   it in a subpackage, so just completely remove the data sub-package,
>   and add the single line "%{_datadir}/%{name}" to the main %files list
> 
Right, I'll do that (I'm going to wait until I can convince camlimages to build
in mock though)
> - Please add an icon, just take any suitable graphics included and resize / edit
>   it as nescesarry then install it under /usr/share/icons/hicolor/<size>/apps
>   and add the necessary scripts to update the icon-cache. (See scriptlets wiki
>   page)
Okay
> 
> 
> p.s. Do you still need a sponsor? Once this and the deps have been reviewed I
> can sponsor you if needed.
> 
Nope, I'm in cvsextras already (was sponsored by Kevin with package windowlab)
and removed the FE-NEEDSPONSOR stuff already (anyway, how could I have approved
your other package? ;))
> 

(In reply to comment #3)
> Right, I'll do that (I'm going to wait until I can convince camlimages to build
> in mock though)

If you need help with this (I can always try) let me know.

> > p.s. Do you still need a sponsor? Once this and the deps have been reviewed I
> > can sponsor you if needed.
> > 
> Nope, I'm in cvsextras already (was sponsored by Kevin with package windowlab)
> and removed the FE-NEEDSPONSOR stuff already (anyway, how could I have approved
> your other package? ;))
> > 
> 

True, back in the old days when we didn't use flags, anyone with bugzilla edit
rights could approve a package. Also you're not in owners.list (I guess that
windowlab isn't approved / imported into cvs yet?) or maybe I just haven't done
a cvs update in my owners dir for a while....

 

(In reply to comment #4)
> (In reply to comment #3)
> > Right, I'll do that (I'm going to wait until I can convince camlimages to build
> > in mock though)
> 
> If you need help with this (I can always try) let me know.
I think I've just got it, I'll leave it for now, and test in mock tonight (I
need to leave for town in a few mins so yeah)
> 
> > > p.s. Do you still need a sponsor? Once this and the deps have been reviewed I
> > > can sponsor you if needed.
> > > 
> > Nope, I'm in cvsextras already (was sponsored by Kevin with package windowlab)
> > and removed the FE-NEEDSPONSOR stuff already (anyway, how could I have approved
> > your other package? ;))
> > > 
> > 
> 
> True, back in the old days when we didn't use flags, anyone with bugzilla edit
> rights could approve a package. Also you're not in owners.list (I guess that
> windowlab isn't approved / imported into cvs yet?) or maybe I just haven't done
> a cvs update in my owners dir for a while....
Quite possible, but the merge is happening now
https://www.redhat.com/archives/fedora-extras-commits/2007-April/msg02380.html
> 
>  
> 
> 

Oh, I meant to say as well, that I'll have updated packages in about 18 hours
time (if camlimages keeps a sane head).

Spec URL: http://dev.nigelj.com/SRPMS/freetennis.spec
SRPM URL: http://dev.nigelj.com/SRPMS/freetennis-0.4.8-2.src.rpm

(Uploading now)

Still rpmlint clean, changelog:

* Thu May 03 2007 Nigel Jones <dev> 0.4.8-2
- Add freetennis.png and alter freetennis.desktop to show icon
- Change builddep to ocaml-SDL-devel

Spec URL: http://dev.nigelj.com/SRPMS/freetennis.spec
SRPM URL: http://dev.nigelj.com/SRPMS/freetennis-0.4.8-3.src.rpm

Only change is camlimages->ocaml-camlimages

MUST:
=====
* rpmlint output is clean
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel i386
* BR: ok
* No locales
* No shared libraries, ldconfig not needed
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package as needed
* no .desktop file needed

MUST FIX
========
* Change Source0 to the generic sf download url, and use %{version}
* Change "Categories=Game" to "Categories=Game;SportsGame;" in the .desktop
  file, so that it will show up under the Games->"Sport Games" submenu for
  people who have the games-menus package installed (try it).

Spec URL: http://dev.nigelj.com/SRPMS/freetennis.spec
SRPM URL: http://dev.nigelj.com/SRPMS/freetennis-0.4.8-1.src.rpm

Update to include MUST FIX items

All MUST FIX items handled, approved!

(In reply to comment #11)
> All MUST FIX items handled, approved!
> 

Thanks,

New Package CVS Request
=======================
Package Name:      freetennis
Short Description: Tennis simulation game
Owners:            dev
Branches:          FC-6 devel
InitialCC:         <empty>

Excluding arch ppc64 so it will build in F7 (already built on FC6), for those
interested, the new bug is Bug #239521

Closing...