MUST items:
- Package meets naming and packaging guidelines - OK
- Spec file matches base package name. - OK
- Spec has consistent macro usage. - OK
- Meets Packaging Guidelines. - ***

Summary should probably drop the leading 'A'.

You appear to be packaging a git snapshot. (In fact, you're including the entire .git directory in the tarball, which isn't really needed.) 

Please see https://fedoraproject.org/wiki/Packaging/SourceURL for how to handle this, and how to version the package.

- License  - LGPLv2+ - OK
- License field in spec matches - ***

"GPLv2+ and LGPLv2+"

I don't actually see any GPLv2+ code in the tarball.

- License file included in package - OK
- Spec in American English - OK
- Spec is legible. - OK
- Sources match upstream md5sum: - ***

See above re: snapshot packaging.

- Package needs ExcludeArch - Possibly pointless on s390, but don't really need to exclude it - OK
- BuildRequires correct  - OK
- Spec handles locales/find_lang - N/A
- Package is relocatable and has a reason to be. - N/A
- Package has %defattr and permissions on files is good. - OK
- Package has a correct %clean section. - OK
- Package has correct buildroot - OK
- Package is code or permissible content. - OK
- Doc subpackage needed/used. - OK
- Packages %doc files don't affect runtime. - OK

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

- Package compiles and builds on at least one arch. - Tested x86_64 (w/mock)
- 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. - OK
- No rpmlint output.

libiphone-devel.x86_64: W: no-documentation

Barring building the docs with doxygen, nothing to add here.

- final provides and requires are sane:

Looks good.

SHOULD Items:

- Should build in mock. - OK
- Should function as described. - No hardware, can't test
- Should have sane scriptlets. - OK
- Should have subpackages require base package with fully versioned depend. - OK
- Should have dist tag - OK
- Should package latest version - ***

See above re: source control pulls.

So, for approval:
- fix %{version} and source control URL to specify what revision you're pulling
- fix License: tag
- maybe tweak summary

If this is going to change ABI frequently without changing soname, a warning in the -devel package might be nice. Then again, if nothing other than the FUSE client is going to use the library, it may not be relevant.

Additionally to changing the state to ASSGINED and assigned to the bug to onself, fedora‑review should be set to "?" if a reviewer wants to review a request. I do this now.

(In reply to comment #1)
> - Meets Packaging Guidelines. - ***
> 
> Summary should probably drop the leading 'A'.

Will do

> You appear to be packaging a git snapshot. (In fact, you're including the
> entire .git directory in the tarball, which isn't really needed.) 
> 
> Please see https://fedoraproject.org/wiki/Packaging/SourceURL for how to handle
> this, and how to version the package.

I missed that. Will update it.
 
> - License  - LGPLv2+ - OK
> - License field in spec matches - ***
> 
> "GPLv2+ and LGPLv2+"
> 
> I don't actually see any GPLv2+ code in the tarball.

There was a COPYING and COPYING.LESSER file included which is why I marked it as such. If you think the COPYING file is irrelevant.

> - License file included in package - OK
> - Spec in American English - OK
> - Spec is legible. - OK
> - Sources match upstream md5sum: - ***
> 
> See above re: snapshot packaging.

ACK.

> - Should package latest version - ***
> 
> See above re: source control pulls.

ACK.

> So, for approval:
> - fix %{version} and source control URL to specify what revision you're pulling
> - fix License: tag
> - maybe tweak summary
> 
> If this is going to change ABI frequently without changing soname, a warning in
> the -devel package might be nice. Then again, if nothing other than the FUSE
> client is going to use the library, it may not be relevant.

Would the warning be contained in an included text file or in the description or somewhere else? I'm not sure whether the library would be used by conduit or rhythmbox or whether they'd use the ifuse client.

I'll update the file with the suggestions at update the ticket once complete.

(In reply to comment #3)
> > - License  - LGPLv2+ - OK
> > - License field in spec matches - ***
> > 
> > "GPLv2+ and LGPLv2+"
> > 
> > I don't actually see any GPLv2+ code in the tarball.
> 
> There was a COPYING and COPYING.LESSER file included which is why I marked it
> as such. If you think the COPYING file is irrelevant.

What's listed in the headers of the code trumps whatever COPYING files are in the package itself.

> > If this is going to change ABI frequently without changing soname, a warning in
> > the -devel package might be nice. Then again, if nothing other than the FUSE
> > client is going to use the library, it may not be relevant.
> 
> Would the warning be contained in an included text file or in the description
> or somewhere else? I'm not sure whether the library would be used by conduit or
> rhythmbox or whether they'd use the ifuse client.

Maybe just in the README, if it's not already there.

If you don't know of any other users of the library, it's not really a big deal.

SPEC: http://pbrobinson.fedorapeople.org/libiphone.spec
SRPM: http://pbrobinson.fedorapeople.org/libiphone-0.1.0-2.git8c3a01e.fc10.src.rpm

Updated based on review. I think I've done the details on pulling the specific git version (its currently based on HEAD).

Removed the GPLv2+ tag and the COPYING file that specifies it as well, and the other listed bits.

You've got two git-clone lines in the spec.

For making the tarball, you may want to do something like:

git-archive --format=tar --prefix libiphone-%{version}/ <ref> | gzip > libiphone-%{version}.tar.gz


For the version, you need to prefix it with a date, otherwise it won't sort right when the ref changes. xorg-x11-drv-nouveau has an example of this.

Thanks, they were the commands I couldn't find late last night. Third time lucky I think. Spec in same location. SRPM: http://pbrobinson.fedorapeople.org/libiphone-0.1.0-3.20081201git8c3a01e.fc10.src.rpm

Looks good. APPROVED.

New Package CVS Request
=======================
Package Name: libiphone
Short Description: Library for connecting to Apple iPhone and iPod touch
Owners: pbrobinson
Branches: F-9 F-10
InitialCC:

cvs done.

Built and in rawhide. Thanks for the review.