Bug 199402

You should add '-%(%{__id_u} -n)' at the end of BuildRoot definition.

I haven't been sponsored yet, so this is not official review.

MUST items:
	* rpmlint doesn't show anything.
	* package is named according to the Package Naming Guidelines.
	* spec file is named correctly.
	* package is licensed with an open-source GPL license.
	* the License field in spec matches the actual license.
	* license file is included in %doc.
	* spec file is legible.
	* package succesfully compiles on i386.
	* there is no need to any build dependencies - package successfully 
compile on
mock.
	* there is no locales.
	* there is no shared library files.
	* there is no duplicate files in %files section.
	* %files section includes %defattr line.
	* package has %clean section.
	* macros are used properly.
	* there is no need to -doc subpackage.
	* files in %doc don't affect the runtime of the application.
	* there is no GUI applications.
COMMENTS:
	* I cannot check if sources match md5sum because I cannot connect to 
ftp.hungry.com server.
	* BuildRoot should be: %{_tmppath}/%{name}-%{version}-%{release}-
root-%(%{__id_u} -n)
	* why do you use 'make' instead of 'make %{?_smp_mflags}'? According to 
Parallel
make chapter of Packaging Guidelines you should use the second option.

I agree with my pre-reviewer :-) Only the using of %{_smp_flags} is not much
useful when compiling 4 small C files.

Review:
- no rpmlint output on any package
- package name OK
- spec file name OK, is in English and is legible
- package meets the Packaging Guidelines
- license OK (GPL) and is included
- ?source matches upstream? - couldn't be checked due the problems of ftp.hungry.com
- compiles and builds at least on i386 (FC4 and devel)
- no BuildRequires needed
- no localized files
- has no shared lib
- do not create any directory
- no duplicates files, permissions are set properly, uses %defattr
- has %clean section
- consistent use of macros
- contains code
- no large docs, %doc is not required during runtime
- no devel subpackage required, no pkgconfig file
- no .la libtool archives
- not a GUI application
- it works

APPROVED, when you fix the BuildRoot

Michael, you don't need to be sponsored to review a package, you only need to
open up an account on admin.fedora.redhat.com/accounts/

On the comments by both (thanks for the fast replies!):
o ftp.hungry.com has been often bad for me, too. Have you tried accessing the file 
  (not the folder) directly?
o BuildRoot as quoted is the "preferred buildroot" which doesn't really make
  sense. I submitted a request to review the guidelines on this. But as a
  _preferred_ entry this is considered a SHOULD, not MUST. 
o %{?_smp_mflags} should be used IMO if one really knows the package builds as 
  such. I have been bitten by too many Makefiles that didn't build in parallel.
  As the ordering is non-deterministic there is noooo way to find out other then
  reviewing the Makefiles themselves. That may make sense on large packages of
  the size of openoffice, but for a tiny package the review and risk of Makefile
  bugs isn't worth the few CPU cycles.

(In reply to comment #4)
> Michael, you don't need to be sponsored to review a package, you only need to
> open up an account on admin.fedora.redhat.com/accounts/

I have written it to take note of I'm not sponsored ;-) maybe someone will have 
a look to one of my packages.

> o ftp.hungry.com has been often bad for me, too. Have you tried accessing the 
file 
>   (not the folder) directly?

Yes, wget output:

Resolving ftp.hungry.com... 199.181.107.40
Connecting to ftp.hungry.com|199.181.107.40|:21... connected.
Logging in as anonymous ... Logged in!
==> SYST ... done.    ==> PWD ... done.
==> TYPE I ... done.  ==> CWD /pub/hungry/chrpath ... done.
==> PASV ... couldn't connect to 199.181.107.40 port 58316: No route to host

I have tried both wget and ftp and I have the same problems. Also have played
with passive/no passive. No success yet.

Because the BuildRoot is only suggested, the package is APPROVED even when you
disagree with me/us.

(In reply to comment #5)
> (In reply to comment #4)
> > Michael, you don't need to be sponsored to review a package, you only need >
> > to open up an account on admin.fedora.redhat.com/accounts/
> 
> I have written it to take note of I'm not sponsored ;-) maybe someone will
> have a look to one of my packages.

OK, I understand now. I'd take a look, but I'm not a sponsor. :/
Ping me for the second package to get a review for.

(In reply to comment #6)
> Because the BuildRoot is only suggested, the package is APPROVED even when you
> disagree with me/us.

Thanks!

(In reply to comment #7)
> OK, I understand now. I'd take a look, but I'm not a sponsor. :/
> Ping me for the second package to get a review for.

My packages are Bug 199192 and Bug 198878. More important to me is the first 
one.

Thanks, packages for FC4-FC6 have been built and will appear soon.

I saw you got sponsored on the second bug, so I can look into the first one.