REVIEW for 
9ed6f1ac2aa7ab76e77a92fbe3f3763f  sing-1.1-1.src.rpm

TODO - MUST: rpmlint must be run on every package. The output should be posted in the review: Doesn't build, so no binaries.
OK - MUST: The package is named according to the Package Naming Guidelines.
OK - MUST: The spec file name matches the base package %{name}, in the format %{name}.spec.
OK - MUST: The package meets the Packaging Guidelines.
OK - MUST: The package is licensed with a Fedora approved license and meets the Licensing Guidelines: GPLv2+
OK - MUST: The License field in the package spec file matches the actual license.
OK - MUST: The license file from the source package is included in %doc.
OK - MUST: The spec file is in American English.
OK - MUST: The spec file for the package is legible.
OK - MUST: The sources used to build the package match the upstream source by MD5 f9f649c4b40174a983601d46e4a3daac
FAIL - MUST: The package does not successfully compile and build into binary rpms on i386
N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
OK - MUST: All build dependencies are listed in BuildRequires.
N/A - MUST: The spec file handles locales properly with the %find_lang macro.
N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package.
OK - MUST: The package owns all directories that it creates: none exept in %{_docdir}
OK - MUST: The package does not contain any duplicate files in the %files listing.
OK - MUST: Permissions on files are set properly. The %files section includes a %defattr(...) line.
OK - MUST: The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT.
OK - MUST: The package consistently uses macros, as described in the macros section of Packaging Guidelines.
OK - MUST: The package contains code, or permissable content.
N/A - MUST: Large documentation files should go in a -doc subpackage.
OK - MUST: Files included as %doc do not affect the runtime of the application.
N/A - MUST: Header files must be in a -devel package.
N/A - MUST: Static libraries must be in a -static package.
N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
N/A - MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package.
N/A - MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section.
OK - MUST: The packages does not own files or directories already owned by other packages.
OK - MUST: At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.
OK - MUST: All filenames in rpm packages are valid UTF-8.


SHOULD Items:
N/A - 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.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
FAIL - SHOULD: The the package builds in mock: Rawhide buildlog attached
FAIL - SHOULD: The package should compile and build into binary rpms on all supported architectures.
TODO - SHOULD: The package functions as described.
N/A - SHOULD: If scriptlets are used, those scriptlets must be sane.
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
N/A - SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg.
OK - SHOULD: The package has a file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin: {_includedir}/pcap.h, but for BuildRequires this is ok to allow easier maintainance cross several Fedora releases.


Issues:
Does not build in i386 rawhide mock, log attached
Does not build on my i386 F-10, log attached
Docs: Change
  %doc AUTHORS ChangeLog COPYING LEEME README THANKS
to
  %doc AUTHORS ChangeLog COPYING README THANKS
  %lang(es) %doc LEEME


Notes:
Macros: IMO you are using too much macros. I would prefer not having macros in URL and Source0 (except %{version} of course).
Description:
  "Sing is a little tool that sends ICMP packets fully customized from command line."
should IMO be changed
  "Sing is a little tool that sends fully customized ICMP packets from command line."

Created attachment 340265 [details]
buildlog of a failed local F10 build on i386

Created attachment 340266 [details]
buildlog of a failed rawhide mock build on i386

Looks like your Rawhide build in mock is Fedora 11 based. Because at real
Rawhide, Fedora 12 based, the package builds. On Fedora 10, I'm waiting for
https://admin.fedoraproject.org/updates/libnet10-1.0.2a-17.fc10 to make it
into the repositories. Same for Fedora 9. For Fedora 11, I'm waiting for a
decision at https://fedorahosted.org/rel-eng/ticket/1538.

Local build fails if too many autotool versions are installed in parallel,
but a successful local build never have been a must, haven't it?

De-macrofying the URL tag is possible, if that makes you happy. Other issue
as well as notes are minor so far to me, right?

http://koji.fedoraproject.org/koji/taskinfo?taskID=1308472 is a working
scratch build in real Rawhide, Fedora 12 based. Package should be working
on Fedora 10/11, if you install the latest libnet10 from URL above.

Ok, now that the update got pushed mockbuilds work for me, even in the wannabe rawhide mock that turned out to be F11.

(In reply to comment #4)
> Local build fails if too many autotool versions are installed in parallel,
> but a successful local build never have been a must, haven't it?

http://fedoraproject.org/wiki/Packaging/ReviewGuidelines#cite_ref-6
"MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture."
IMO this also includes local builds. Anyway, the package must no break if more than one version of autotool is installed. How about:

  for file in %{_datadir}/automake-*/config.*; do
      cp -f $file .
  done

Works fine here.

> De-macrofying the URL tag is possible, if that makes you happy. Other issue
> as well as notes are minor so far to me, right?  

Yes, they are minor non-blockers, but that doesn't mean the package can't be improved, right?

The outstanding issues:
OK - rpmlint /var/lib/mock/fedora-rawhide-i386/result/sing-*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.
OK - The package successfully compiles and builds into binary rpms on i386 (with the change I suggested)

As there are no blockers left, the package is APPROVED. Nevertheless I strongly advice you to also apply minor corrections and improvements fro this review. TIA.

Please consider https://fedoraproject.org/wiki/Packaging_tricks#Use_of_common_namespace before using a 4 letter binary name.

So what do you suggest? Work with upstream and rename the package wont work as there is no upstream any longer.

I don't think the length will be an issue (we even have packages like gdm). I searched freshmeat and sourceforge and did not find anything that interfered, even in a wider sense.

(In reply to comment #8)
> So what do you suggest? Work with upstream and rename the package wont work as
> there is no upstream any longer.
Keep the name of the package but invent something else for the binary. sing-icmp for instance .


> I don't think the length will be an issue (we even have packages like gdm). I
> searched freshmeat and sourceforge and did not find anything that interfered,
> even in a wider sense.  
I am very very very sure that noone will ever write a music player and name it "sing" ( because "play" is already taken, you know...). I am also very sure that no one will ever try to package that shiny new application for fedora :)

Christoph, I didn't question that I need to apply your suggestions and 
enhancements. I just wanted to make sure, that I don't need to create a
new srpm for these minor changes. Sorry, if I was unclear. I'll have a
look into the Packaging Tricks as well, before importing into CVS. But
I am now requesting CVS branching, because it also takes some time - at
least usually.


New Package CVS Request
=======================
Package Name: sing
Short Description: Sends fully customized ICMP packets from command line
Owners: robert
Branches: EL-4 EL-5 F-9 F-10 F-11
InitialCC:

(In reply to comment #10)
> I'll have a look into the Packaging Tricks as well, before importing into CVS.

No, _after_ import. Please import the package as is and then make the changes, so the community can follow them on the commits list.

cvs done.