Needs work:
* the conf files in /etc/shorewall should be set with flag noreplace
* /usr/share/shorewall dir should be 755 and the files in it should be readable,
since they are not conf files there is no point in making them 600.
* the service is enabled by default, please run a substitution on
/etc/init.d/shorewall to change the chkconfig line
* Requirements: missing dependancy on chkconfig in %post and %preun
* Scriptlets: missing "service" command in %preun and %postun 
  (wiki: ScriptletSnippets)
* In the files list, /sbin should be replaced with %{_sbindir}

Minor:
* remove empty %build section
* %{_prefix}/share should be replaced with %{_datadir}

%{_sbindir} != /sbin 

(In reply to comment #2)
> %{_sbindir} != /sbin 

Ooops, right :)
Please forget about this one.

Thanks for the review. It has been updated... see

http://www.marcanoonline.com/downloads/fedora/package_submissions/shorewall/shorewall.spec
http://www.marcanoonline.com/downloads/fedora/package_submissions/shorewall/shorewall-2.4.4-3.src.rpm

I disabled the service autostart using a patch and no a substitution. I think is
more easy to detect when the source init script is changed when the patch does
not applies anymore

Well I still have a few warnings.. It is my first time running rpmlint :-(

> W: shorewall no-version-in-last-changelog
My mistake

> E: shorewall non-standard-dir-perm /var/lib/shorewall 0700
> E: shorewall non-readable /var/lib/shorewall/proxyarp 0600
> E: shorewall non-readable /var/lib/shorewall/nat 0600
> E: shorewall non-readable /var/lib/shorewall/chains 0600
> E: shorewall non-readable /var/lib/shorewall/zones 0600
> E: shorewall non-readable /var/lib/shorewall/restarted 0600
This files are generated by shorewall, and some are shell scripts that must be
hidden to non root users. When they do not exist, shorewall creates them with
0600. I think that the author of the original SPEC file add them empty in order
to be sure they are removed when uninstalled

> E: shorewall non-standard-executable-perm /sbin/shorewall 0554
Will be fixed to 0774

> E: shorewall non-executable-script /usr/share/shorewall/firewall 0644
> E: shorewall non-executable-script /usr/share/shorewall/functions 0644
> E: shorewall non-executable-script /usr/share/shorewall/help 0644
This scripts are sourced by other shell scripts, they are not called directly,
so I will let them non-executable

> W: shorewall no-reload-entry /etc/rc.d/init.d/shorewall
Will be fixed

> E: shorewall subsys-not-used /etc/rc.d/init.d/shorewall
What this means?

(In reply to comment #5)
> > E: shorewall subsys-not-used /etc/rc.d/init.d/shorewall
> What this means?

Regular daemons should write their PID in a file in /var/lock/subsys. In
shorewall's case, you can ignore it.
 

new update:

http://www.marcanoonline.com/downloads/fedora/package_submissions/shorewall/shorewall.spec
http://www.marcanoonline.com/downloads/fedora/package_submissions/shorewall/shorewall-2.4.4-4.src.rpm

Review for release 4:
* RPM name is OK
* Source shorewall-2.4.4.tar.bz2 is the same as upstream
* This is the latest version
* Builds fine in mock
* rpmlint of shorewall looks OK
* File list of shorewall looks OK
* Works fine.
APPROVED

Updated to 2.4.5

Review for release 1:
* Changes are OK
* Source shorewall-2.4.5.tar.bz2 is the same as upstream
Still APPROVED.

Build logs:

http://buildsys.fedoraproject.org/logs//development/1809-shorewall-2.4.5-2.fc5/
http://buildsys.fedoraproject.org/logs//4/1810-shorewall-2.4.5-2.fc4/

is this the last step in order to push it to the repositories?

Yes, the package is in the repos now. Thanks for maintaining it.

Please create a EL6 branch.

There is already a EL-6 branch. ;)

Oh yes, so there is, apologies. I'll blame early onset Alzheimers.:)