This is my third package, and I am seeking a sponsor.

* Project home page URL is  http://linuxbrit.co.uk/scrot/
  (note the /scrot/ postfix)


* rpmlint scrot-0.8-1.fc5.src.rpm 
W: scrot non-standard-group Applications/Graphics

Better:  User Interface/X


* License is MIT -- not BSD.
  cf. http://www.opensource.org/licenses/mit-license.html


* rpmlint /home/qa/tmp/rpm/RPMS/scrot-0.8-1.i386.rpm
W: scrot non-standard-group Applications/Graphics
W: scrot no-version-in-last-changelog

Although rpmlint doesn't understand where you've put the "0.8"
version in the %changelog, you should add the release value in there,
too:

* Tue Sep 26 2006 Michael Rice <errr[AT]errr-online.com>
- 0.8-1
- Initial RPM release

Or:

* Tue Sep 26 2006 Michael Rice <errr[AT]errr-online.com> - 0.8-1
- Initial RPM release

Or:

* Tue Sep 26 2006 Michael Rice <errr[AT]errr-online.com> 0.8-1
- Initial RPM release


* Documentation duplicated because of a wrong %patch. Keep the
%doc lines in the spec, get rid of %_datadir/%name:

$ rpmls -p /home/qa/tmp/rpm/RPMS/scrot-0.8-1.i386.rpm
-rwxr-xr-x  /usr/bin/scrot
drwxr-xr-x  /usr/share/doc/scrot-0.8
-rw-r--r--  /usr/share/doc/scrot-0.8/AUTHORS
-rw-r--r--  /usr/share/doc/scrot-0.8/COPYING
-rw-r--r--  /usr/share/doc/scrot-0.8/ChangeLog
-rw-r--r--  /usr/share/doc/scrot-0.8/README
-rw-r--r--  /usr/share/doc/scrot-0.8/TODO
-rw-r--r--  /usr/share/man/man1/scrot.1.gz
drwxr-xr-x  /usr/share/scrot
-rw-r--r--  /usr/share/scrot/AUTHORS
-rw-r--r--  /usr/share/scrot/ChangeLog
-rw-r--r--  /usr/share/scrot/README
-rw-r--r--  /usr/share/scrot/TODO


* ./src/Makefile.am sets a bad INCLUDES line. First of all, standard
search path for headers should not be re-added with -I as this breaks
customised search paths. And second, be careful that in updates (or
other packages) the -O3 does never come after our global optflags.

Michael, thanks for going over this package for me. I have redone some things,
and now rpmlint is no longer giving any output. I think I have fixed all the
things you pointed out with the exception of 

* ./src/Makefile.am sets a bad INCLUDES line. First of all, standard
search path for headers should not be re-added with -I as this breaks
customised search paths. And second, be careful that in updates (or
other packages) the -O3 does never come after our global optflags.

I am not sure exactly what to do here with the Makefile.am  I also have no idea
what you mean about the -O3 Would you be able to provide me with some more input?

Thanks.

http://errr.fluxbox-wiki.org/fedora_stuff/scrot/2/scrot.spec
http://errr.fluxbox-wiki.org/fedora_stuff/scrot/2/scrot-0.8-2.fc5.src.rpm

In src/(In reply to comment #3)
 
> I am not sure exactly what to do here with the Makefile.am  I also have no idea
> what you mean about the -O3 Would you be able to provide me with some more input?

If you look at the compilation, you'll notice that there is
   gcc -DHAVE_CONFIG_H -I. -I. -I. -g -O3 -Wall
and the RPM_OPT_FLAGS options on the same line 
   -O2 -g -pipe -Wall   -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
   -fstack-protector   --param=ssp-buffer-size=4 -m32 -march=i386 
   -mtune=generic -fasynchronous-unwind-tables
In that case the RPM_OPT_FLAGS (and specifically -O2) override the
other flags hardcoded in Makefile.am (and specifically -O3). That's
right, but the flags -g -O3 -Wall shouldn't have been set in the
first place. The upstream is broken, and should be notified that
those flags appearing in src/Makefile.am

INCLUDES          = -g -O3 -Wall -I/usr/X11R6/include \
$(X_CFLAGS) -I$(prefix)/include -I$(includedir) -I. \
-DPREFIX=\""$(prefix)"\" @GIBLIB_CFLAGS@

should be removed, (together with some -I which shouldn't be 
set here) such that the line looks like something along
   INCLUDES          = $(X_CFLAGS) -DPREFIX=\""$(prefix)"\" @GIBLIB_CFLAGS@
(or
   INCLUDES          = $(X_CFLAGS) -DPREFIX=\""$(prefix)"\" $(GIBLIB_CFLAGS)

Right. That was just a hint, a recommendation to be careful during
package review, since this can be an issue also with other packages.
And sometimes it changes unexpectedly when upstream modifies the
Makefile setup (e.g. our optflags might get overruled). Here it is
not a MUST FIX item. Just something to comment on.

> I/usr/X11R6/include -I/usr/include -I/usr/include

To expand on that, /usr/include is a standard search path for include
files. It need not be added to the user search list with -I again. It
is harmless with this package, but with other packages it can happen
that the added -I/usr/include enters packaged files and propagates to
other packages via build requirements. As a worst-case, the modified
search order for include files makes it impossible to customise the
search path further in order to locate the desired headers. In other
words, standard search locations suddenly override user-defined locations,
boom. This is a common mistake by some upstream projects.

So then is this package done or do I still need to do something else to it.. I
have reported this issue upstream for the software author to review it, anything
else or is this one at a stoping point?

The INCLUDES issue is not a blocker.

Regarding BuildRequires,
giblib-devel depends on imlib2-devel and imlib2-devel depends on
libX11-devel  

The README is useless since it contains only build instructions.

For the %description, I would have chosed what is on the home page, 
(in my opinion this is not a blocker):

   scrot is a commandline screen capture util like "import", but using 
   imlib2.

   It has lots of options for autogenerating filenames, and can do fun 
   stuff like taking screenshots of multiple displays and glueing them 
   together.

I don't have any other comment. The final word belong to Michael of 
course.

How do you track what depends on what so that I can track these things before I
submit in the future??

I just do rpm -q --requires giblib-devel.
Or I try to remove the package I suspect other packages depend 
on and I watch rpm error or yum remove dependencies resolution.

* Eliminating redundant BuildRequires is _not_ a MUSTFIX item and
sometimes can be tiresome and not worthwhile:

./src/scrot.h contains _direct_ dependencies on X headers, so an X
-devel rpm in the BuildRequires makes sense (even if it may be
redundant in the complete dependency-chain).

There is no direct dependency on Imlib2, since giblib is a wrapper
library for it and giblib-devel "Requires: imlib2-devel" already.
"BuildRequires: imlib2-devel" can be removed safely.


* I agree that the README in %doc can be deleted. It is irrelevant to
the rpm package user and hasn't changed in six years. Same applies to
the TODO file, which contains nothing of interest. These files can be
excluded after importing the package to CVS, though. And note that for
more active software projects, it might happen that an upgrade adds
interesting contents to either README or TODO ;) and a packager might
forget about re-adding them.


* APPROVED: scrot-0.8-2.fc5.src.rpm

I'll sponsor Michael. So, please Michael go through the steps 
to create a CLA. I hope you allready read
http://fedoraproject.org/wiki/Extras/Contributors

you should now proceed to
http://fedoraproject.org/wiki/Extras/Contributors#head-a89c07b5b8abe7748b6b39f0f89768d595234907
and I'll sponsor you.