Some quick notes:

You should probably use User Interface/Desktops, instead of Applications/System
for the group.  Most of the other nautilus plugins (nautilus-image-converter,
nautilus-open-terminal) use this.

I believe you can also drop the Requires for nautilus, since the BR on this
should automatically pull this in.

If I've got some extra time tomorrow, I'll do a proper review.

NEEDSWORK

MD5Sums:
682d452e4b05c77b5a258c7ec49634bc  nautilus-actions-0.99.tar.gz

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* Desktop entry is fine
* All directories are owned by this or other packages
* No deprecated fields used
* All necessary BuildRequires listed.
* All desired features are enabled
* Package rebuilds as non-root user
* Make succeeds even when %{_smp_mflags} is defined
* Files have appropriate permissions and owners

Needswork:
* In the %pre section, you scriptlet needs to be changed, the [NAME] should be
changed to config_newaction.

* Rpmlint errors:
E: nautilus-actions description-line-too-long
W: nautilus-actions conffile-without-noreplace-flag
/etc/gconf/schemas/config_newaction.schemas
W: nautilus-actions devel-file-in-non-devel-package /usr/bin/nautilus-actions-config
W: nautilus-actions devel-file-in-non-devel-package
/usr/bin/nautilus-actions-new-config

* The line where you remove the fedora*.desktop file can be removed, since it
comes before where the desktop file is created & installed.  The
--delete-original switch handles the .desktop file that comes with the source.

* It seems redundant to include both the AUTHORS & MAINTAINERS files, since the
MAINTAINER file only contains their e-mail addresses, which is also included in
the AUTHORS file.  I would suggest not packing the MAINTAINERS file.


Notes:
I'm not running Rawhide, so I'm not able to verify that this runs correctly.  It
does build fine in Mock, though.

I've actually corrected most of the things you pointed out (except for removing
the MAINTAINERS file) in my local version, I just forgot to post it here.
As for the rpmlint warnings, I'm not sure if its really necessary to create a
devel subpackage for this nautilus-actions.

ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-2.src.rpm

Oh, I forgot one other thing.  Is e2fsprogs-devel really needed to build this? 
I don't see anything the configure script that would require this, since
e2fsprogs deals with ext2 filesystems.

Also, it looks like you description in your spec is still to long.

(In reply to comment #4)
> Oh, I forgot one other thing.  Is e2fsprogs-devel really needed to build this?

Yes. I'm quite surprised by it too, building it in a clean mock will fail
without e2fsprogs-devel.
   
> I don't see anything the configure script that would require this, since
> e2fsprogs deals with ext2 filesystems.
> 
> Also, it looks like you description in your spec is still to long.

I'll shorten it in the next release.

Here are comments by package author, Frederic Ruaudel;

> Concerning the -devel rpmlint warnings, it is definitely not necessary
> as the mentioned files are not at all development files.
> nautilus-actions-config is the GUI config tool and
> nautilus-actions-new-config is a command line tool to create a config
> file that can be imported with the GUI tool or directly with
> gconftool-2.
>
> Concerning the requirement for e2fsprogs-devel, it is needed because I
> use the uuid library provided by this package to generate uniq config
> identifiers.

Are there any blockers left for this review?

None (that I can think of), except for one line in the package description
extending too long. That is fixed by the files below.
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-3.src.rpm
Unless Brian spot anything new, the package should be good enough now.

Since I'm not running Rawhide yet, I didn't take this bug for a formal review
(thats why I left it as FE-NEW).  Another reviewer will need to verify that this
package runs as expected before, and then they can give a final approval.  Of
course, until the mesa-libGL package is fixed in Rawhide, they won't be able to
build this package in Mock.

Got the build in mock against rawhide today to actually start.
It did not compile however.
Build.log reports:
checking for     glib-2.0 >= 2.4.0                       gthread-2.0 >= 2.4.0
                 gmodule-2.0 >= 2.4.0                    gtk+-2.0    >= 2.4.0
                 libglade-2.0 >= 2.4.0                   libgnome-2.0 >= 2.7.0
         libgnomeui-2.0 >= 2.7.0                 gconf-2.0 >= 2.8.0     
libnautilus-extension >= 2.8.0... Package xdmcp was not found in the pkg-config
search
path. Perhaps you should add the directory containing `xdmcp.pc' to the
PKG_CONFIG_PATH environment variable Package 'xdmcp', required by 'X11', not found
configure: error: Library requirements (         glib-2.0 >= 2.4.0
         gthread-2.0 >= 2.4.0                    gmodule-2.0 >= 2.4.0
         gtk+-2.0    >= 2.4.0                    libglade-2.0 >= 2.4.0
         libgnome-2.0 >= 2.7.0           libgnomeui-2.0 >= 2.7.0
 gconf-2.0 >= 2.8.0      libnautilus-extension >= 2.8.0) not met; consider
adjusting the PKG_CONFIG_PATH environment variable if your libraries are in a
nonstandard prefix so pkg-config can find them.
error: Bad exit status from /var/tmp/rpm-tmp.29345 (%build)

you definitely need libXdmcp-devel as a buildrequires
I've added that and the build completes.
I'll see if the resulting packages work on my rawhide box

-jef

Okay my nautilus-actions-0.99-4  binary seems to work on my rawhide box just fine.
The preference dialog works.. and my test actions show up in the right click
context menu.

Getting the buildrequires fixed as per the last comment seems to be the only
thing left to do.

-jef

Jef,

The buildrequires for libXdmcp-devel must have been caused by some recent
changes on rawhide, it build fine without it in mock the last time i tried.
I've change the spec file to now buildrequire libXdmcp-devel and also remove the
buildrequires on libSM-devel as it is now required for libgnomeui-devel which is
already in the list. 
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-4.src.rpm

Note I've not tested this build in mock (I'm currently getting some yum error
using it).

Tested the the build in which mock configuration? fc4 or rawhide?  
I did my build on an fc4 box against the rawhide mock root.
mock running from a rawhide will not work... since the bump of yum to 2.5.x.  

But mock -r fedora-development-i386-core on an fc4 box does work to build
packages for a rawhide at the moment. 

I should be able to finish this up tonite. What I'm not clear about is, am I
suppose to do a full formal review or does the review brian did count and I'm
just suppose to confirm the blockers are fixed?

All i know is this is going to be pretty useful little package.

-jef

(In reply to comment #13)
> Tested the the build in which mock configuration? fc4 or rawhide?  
> I did my build on an fc4 box against the rawhide mock root.
> mock running from a rawhide will not work... since the bump of yum to 2.5.x.  
>
> But mock -r fedora-development-i386-core on an fc4 box does work to build
> packages for a rawhide at the moment. 
> 

I tested that this built against Rawhide back in comment #2 (this package will
not build in FC4 because of the modular X BR).  Due to the some of the recent
packages pushed out in the last couple of days in Rawhide, it might not build
right now.  Once Rawhide settles down a bit, it should hopefully build providing
the BR packages haven't changed.  I'm guessing if there is any problems, it will
be the modular X packages since there's a lot of work being done on them.

> I should be able to finish this up tonite. What I'm not clear about is, am I
> suppose to do a full formal review or does the review brian did count and I'm
> just suppose to confirm the blockers are fixed?

You just need to verify that the blockers have been fixed, and that it will
build and run as expected in FC5.

this is maddening... now I'm getting a new build error from mock using your new
src.rpm

Do you need need libXau-devel  as a buildrequires?
-jef

here's the build.log snippet:
checking for     glib-2.0 >= 2.4.0                       gthread-2.0 >= 2.4.0
                 gmodule-2.0 >= 2.4.0                    gtk+-2.0    >= 2.4.0
                 libglade-2.0 >= 2.4.0                   libgnome-2.0 >= 2.7.0
         libgnomeui-2.0 >= 2.7.0                 gconf-2.0 >= 2.8.0     
libnautilus-extension >= 2.8.0... Package xau was not found in the pkg-config
search path. Perhaps you should add the directory containing `xau.pc' to the
PKG_CONFIG_PATH environment variable Package 'xau', required by 'X11', not found
configure: error: Library requirements (         glib-2.0 >= 2.4.0
         gthread-2.0 >= 2.4.0                    gmodule-2.0 >= 2.4.0
         gtk+-2.0    >= 2.4.0                    libglade-2.0 >= 2.4.0
         libgnome-2.0 >= 2.7.0           libgnomeui-2.0 >= 2.7.0
 gconf-2.0 >= 2.8.0      libnautilus-extension >= 2.8.0) not met; consider
adjusting the PKG_CONFIG_PATH environment variable if your libraries are in a
nonstandard prefix so pkg-config can find them.
error: Bad exit status from /var/tmp/rpm-tmp.47387 (%build)

I would hold off on trying to build this in Mock until Rawhide has settled down
a bit.

Okay ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-4.src.rpm  compiles
in mock for core-development today.

And the resulting binary that i get appears to work on my rawhide system. I did
some shallow testing and created some test actions. 

I'm starting the clock on approval. Unless someone comes up with a blocker I'll
approve this 24 hours from now.

-jef

Has the BR's been looked at since Bug #176313 has been fixed?

It appears so... i've successfully built several packages today in mock that
were previously blowing up with libXau oddness.

-jef

I'm sure it builds, but I believe it should be using libX11-devel instead of
libXdmcp-devel.

actually... it might not "need" either at this point. I forgot that the .4 spec
included that extra buildrequires.   Well I'll look at that tonite if you dont
get to it first. 

-jef

(In reply to comment #20)
> I'm sure it builds, but I believe it should be using libX11-devel instead of
> libXdmcp-devel.

The resolution of Bug #176313 now makes requiring either redundant as nautilus
now depends on libX11-devel, and libX11-devel depends on both libXdmcp-devel and
libXau-devel.

Deji

Okay so can you spin up a -5 spec file with the now correct buildrequires. I'll
do a quick build of it in mock and then i'll approve based on that.


-jef

(In reply to comment #23)
> Okay so can you spin up a -5 spec file with the now correct buildrequires.

Done.
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions.spec
ftp://czar.eas.yorku.ca/pub/nact/nautilus-actions-0.99-5.src.rpm

Deji

-5 builds in mock for development  resulting binary installs
approved

I'd forgotten to close this, the package has already been built and released.
Thanks to Jeff and Brian for the reviews.

Deji

Package Change Request
======================
Package Name: nautilus-actions
New Branches: EL-5

cvs done.