Bug 455067

I am a new contributor and I am seeking a sponsor.

As it seems that the current latest version is 1.3.5, would you update srpm first?

Some notes:
- The name of spec file must be "ferm.spec"
- "ferm -" is redundant for Summary.
- "system/firewalls" is not a standard Group (please refer to
  $ rpmlint -I non-standard-group
- Please remove some meaningless release number suffix.
- The license tag "GPL" is not allowed on current Fedora packaging guidelines:
  https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
  https://fedoraproject.org/wiki/Licensing
- Source must be given by full URL:
  https://fedoraproject.org/wiki/Packaging/SourceURL
- "Requires: perl" is redundant. Usually perl (module) related dependencies are
  automatically detected and added to rebuilt binary rpms by rpmbuild.
- Move the lines:
-----------------------------------------------------------------
sed -i 's/PREFIX = /#PREFIX = /' config.mk
sed -i 's/MANDIR = /#MANDIR = /' config.mk
sed -i 's/DOCDIR = /#DOCDIR = /' config.mk
-----------------------------------------------------------------
  to %build or %prep (%prep is preferred for --short-circuit option)
- Please use macros. /usr should be %{_prefix}, for example:
  https://fedoraproject.org/wiki/Packaging/RPMMacros
- Now %defattr(-,root,root,-) is preferred.

Fist, Mamoru Tasaka very thanks for review.

(In reply to comment #2)
> As it seems that the current latest version is 1.3.5, would you update srpm
> first?
Of course! Moreover now last version 2.0.2. I upgrade to it.

> Some notes:
> - The name of spec file must be "ferm.spec"
Ok.
> - "ferm -" is redundant for Summary.
Ok.
> - "system/firewalls" is not a standard Group (please refer to
>   $ rpmlint -I non-standard-group
Thanks. I'm change it to "Applications/System".
But another question is why rpmlint was silent??

> - Please remove some meaningless release number suffix.
Off course. But it is not meaningless for me :)
> - The license tag "GPL" is not allowed on current Fedora packaging guidelines:
>   https://fedoraproject.org/wiki/Packaging/LicensingGuidelines
>   https://fedoraproject.org/wiki/Licensing
Ok, set it into GPLv2.
> - Source must be given by full URL:
>   https://fedoraproject.org/wiki/Packaging/SourceURL
Set to http://ferm.foo-projects.org/download/2.0/%{name}-%{version}.tar.gz
> - "Requires: perl" is redundant. Usually perl (module) related dependencies are
>   automatically detected and added to rebuilt binary rpms by rpmbuild.
I suppose that do not requires external perl modules. Requires: perl removed.
But I can't understand how it will be detected? It is based on standard Makefile, nor perl buildsystem.
> - Move the lines:
> -----------------------------------------------------------------
> sed -i 's/PREFIX = /#PREFIX = /' config.mk
> sed -i 's/MANDIR = /#MANDIR = /' config.mk
> sed -i 's/DOCDIR = /#DOCDIR = /' config.mk
> -----------------------------------------------------------------
>   to %build or %prep (%prep is preferred for --short-circuit option)
Ok. Moved to %prep section.
> - Please use macros. /usr should be %{_prefix}, for example:
>   https://fedoraproject.org/wiki/Packaging/RPMMacros
Done.
> - Now %defattr(-,root,root,-) is preferred.
Instead of %defattr(-,root,root)? Why?
Done.

http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm.spec
http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm-2.0.2-0.fc9.src.rpm

For 2.0.2-0:

* Release number
  - The minimum number which can be used for Release number is 1.
    0 is used only for pre-release tarballs:
    http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release
    (Of course, please change the Release number every time you modify
     your spec file)

* License
  - As far as I verified the source code, the license tag should
    be "GPLv2+".

* rpmlint issue
------------------------------------------------------
ferm.noarch: W: incoherent-version-in-changelog 2.0.2 2.0.2-0.fc10
------------------------------------------------------
  - The last entry of %changelog should contain full EVR (epoch-version-release)
    information like:
------------------------------------------------------
* Fri Aug 22 2008 Pavel Alexeev <Pahan [ at ] Hubbitus [ DOT ] spb [ dOt.] su> - 2.0.2-1
------------------------------------------------------

* Macros usage consistency
  - When using { on macros, please use { for all macros (except for some cases)
    consistently. You use %{_mandir} and %_sbindir , for example.

* Documents
  - Please also add the following files to %doc.
------------------------------------------------------
NEWS
------------------------------------------------------

(In reply to comment #3)
> (In reply to comment #2)
> > - "system/firewalls" is not a standard Group (please refer to
> >   $ rpmlint -I non-standard-group
> Thanks. I'm change it to "Applications/System".
> But another question is why rpmlint was silent??
  - On my system rpmlint warned about this.s

> > - "Requires: perl" is redundant. Usually perl (module) related dependencies are
> >   automatically detected and added to rebuilt binary rpms by rpmbuild.
> I suppose that do not requires external perl modules. Requires: perl removed.
> But I can't understand how it will be detected? It is based on standard
> Makefile, nor perl buildsystem.

  - This is detected by /usr/lib/rpm/perl.req. Try
------------------------------------------------------
$ rpm -ql ferm | /usr/lib/rpm/perl.req
------------------------------------------------------
    For example when perl.req finds the line like:
------------------------------------------------------
use Data::Dumper;
------------------------------------------------------
    (in /usr/sbin/import-ferm), rpmlint adds "Requires: perl(Data::Dumper) to
    binary rpms.

By the way:

-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

(In reply to comment #4)
> For 2.0.2-0:
> 
> * Release number
>   - The minimum number which can be used for Release number is 1.
>     0 is used only for pre-release tarballs:
Hmmm, I'm lose sight of.
I assume now release shoud be 2 (1 should have been previous).

> * License
>   - As far as I verified the source code, the license tag should
>     be "GPLv2+".
Official site ( http://ferm.foo-projects.org/) says: "Licensed under the GPLv2"

> 
> * rpmlint issue
> ------------------------------------------------------
> ferm.noarch: W: incoherent-version-in-changelog 2.0.2 2.0.2-0.fc10
> ------------------------------------------------------
Ok, done.

> * Macros usage consistency
>   - When using { on macros, please use { for all macros (except for some cases)
>     consistently. You use %{_mandir} and %_sbindir , for example.
                                                                               
%_sbindir replaced by %{_sbindir}
Is there any guidelines about it or is it only by the aesthetic considerations?

> * Documents
>   - Please also add the following files to %doc.
> ------------------------------------------------------
> NEWS
> ------------------------------------------------------
Done.

> (In reply to comment #3)
> > (In reply to comment #2)
> > > - "system/firewalls" is not a standard Group (please refer to
> > >   $ rpmlint -I non-standard-group
> > Thanks. I'm change it to "Applications/System".
> > But another question is why rpmlint was silent??
>   - On my system rpmlint warned about this.s
You are using stable version of the rpmlint or rawhide? May be I can tune warning level somewhere?

$ rpm -q rpmlint
rpmlint-0.84-2.fc9.noarch

> 
> > > - "Requires: perl" is redundant. Usually perl (module) related dependencies are
> > >   automatically detected and added to rebuilt binary rpms by rpmbuild.
> > I suppose that do not requires external perl modules. Requires: perl removed.
> > But I can't understand how it will be detected? It is based on standard
> > Makefile, nor perl buildsystem.
> 
>   - This is detected by /usr/lib/rpm/perl.req. Try
> ...
Thank you very much for the explanation.

(In reply to comment #5)
> By the way:
> 
> -------------------------------------------------------------
> NOTE: Before being sponsored:
> 
> This package will be accepted with another few work. 
> But before I accept this package, someone (I am a candidate) 
> must sponsor you.
It would be grateful if you become my sponsor.

> the person who want to be sponsored (like you) 
> are required to "show that you have an understanding 
> of the process and of the packaging guidelines" as is described
> on :
> http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
> 
> Usually there are two ways to show this.
> A. submit other review requests with enough quality.
> B. Do a "pre-review" of other person's review request
>    (at the time you are not sponsored, you cannot do
>    a formal review)
I like more the first way.
I'm have submitted several packages for review at this time (all waiting review):
php-pecl-runkit - https://bugzilla.redhat.com/show_bug.cgi?id=455226
php-pecl-parsekit - https://bugzilla.redhat.com/show_bug.cgi?id=455227
DivFix++ https://bugzilla.redhat.com/show_bug.cgi?id=458338
xls2csv https://bugzilla.redhat.com/show_bug.cgi?id=458866

Additionally, dependency from decision of Ankur Shrivastava in I may co-maintain axel - https://bugzilla.redhat.com/show_bug.cgi?id=454980 or not. By the way you may see my spec in the request.

Additionally, it is not formal review request (I'm not first submitter), but request of package sim - https://bugzilla.redhat.com/show_bug.cgi?id=411881 also created from my src.rpm and thus packages long time distributed from main website of sim (http://sim-im.org/wiki/Download)

Furthermore, I'm own small rpm-repository for Fedora: http://hubbitus.net.ru/rpm/ and planing submit more few package review requests shortly.

I do not have pre-review, but for that aim may also provide this bug-report https://bugzilla.redhat.com/show_bug.cgi?id=459124

Oh, excuse me, I forgot links:
http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm.spec
http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm-2.0.2-2.fc9.src.rpm

For 2.0.2-2:

(In reply to comment #6)
> (In reply to comment #4) 
> > * License
> >   - As far as I verified the source code, the license tag should
> >     be "GPLv2+".
> Official site ( http://ferm.foo-projects.org/) says: "Licensed under the GPLv2"

  - Well I assume the site you quoted is using wrong license tag, because
    - We guess under what license the package is released by checking the whole
      codes in the tarball
    - Some files in the tarball declares explicitly the license is under GPLv2+
      (see files under doc/).
    (I guess the upstream are using "GPLv2" on the site with the meaning
     of "GPL version 2 and any later" :) )

    However as "GPLv2" is more strict than "GPLv2+", for now I accept "GPLv2"
    license tag. However please ask the upsteam which is correct.

> > * Macros usage consistency
> >   - When using { on macros, please use { for all macros (except for some cases)
> >     consistently. You use %{_mandir} and %_sbindir , for example.
> 
> %_sbindir replaced by %{_sbindir}
> Is there any guidelines about it or is it only by the aesthetic considerations?

  - Only cosmetic issue

> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > - "system/firewalls" is not a standard Group (please refer to
> > > >   $ rpmlint -I non-standard-group
> > > Thanks. I'm change it to "Applications/System".
> > > But another question is why rpmlint was silent??
> >   - On my system rpmlint warned about this.s
> You are using stable version of the rpmlint or rawhide? May be I can tune
> warning level somewhere?
> 
> $ rpm -q rpmlint
> rpmlint-0.84-2.fc9.noarch
  - rpmlint-0.84-2.fc10.noarch. Maybe the dependent packages are related.


Now:
- This package itself is okay 
- Your other review requests seem good to some extent.
  (There are some apparent issues which need fixing, however I don't guess I can have a time
   to review other review requests of yours as I am already reviewing 20 requests...
   I hope someone else will review other requests of yours.)

-----------------------------------------------------------------------------
      This package (ferm) is APPROVED by mtasaka
-----------------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".
As I found your name in FAS I am sponsoring you now.

If you want to import this package into Fedora 8/9, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

(In reply to comment #9)
>     However as "GPLv2" is more strict than "GPLv2+", for now I accept "GPLv2"
>     license tag. However please ask the upsteam which is correct.
I have sent mail to one of main developer with this question.

>   - rpmlint-0.84-2.fc10.noarch. Maybe the dependent packages are related.
Maybe, maybe. I using most packages form F9 stable repository at this time.

> 
> 
> Now:
> - This package itself is okay 
> - Your other review requests seem good to some extent.
>   (There are some apparent issues which need fixing, however I don't guess I
> can have a time
>    to review other review requests of yours as I am already reviewing 20
> requests...
>    I hope someone else will review other requests of yours.)
I hope too. Thank you once more for this review.


> Please follow the procedure written on:
> http://fedoraproject.org/wiki/PackageMaintainers/Join
> from "Install the Client Tools (Koji)".
> As I found your name in FAS I am sponsoring you now.
I have done this steps several times ago. Even in this present reviews I check it builds in koji.

My name in FAS is "hubbitus".

> If you want to import this package into Fedora 8/9, you also have
> to look at
> http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
Ok, thanks, this document I have not read yet.


> If you have questions, please ask me.
Thanks. I think the questions yet to be. What is preferred way to contact with you?

(In reply to comment #10)
> > Please follow the procedure written on:
> > http://fedoraproject.org/wiki/PackageMaintainers/Join
> > from "Install the Client Tools (Koji)".
> > As I found your name in FAS I am sponsoring you now.
> I have done this steps several times ago. Even in this present reviews I check
> it builds in koji.
> 
> My name in FAS is "hubbitus".

I am already sponsoring you. Please follow "Join" wiki again.
From what you say, I guess next step of yours is
"Add Package to CVS and Set Owner" (i.e. write CVS request on this bug)

> > If you have questions, please ask me.
> Thanks. I think the questions yet to be. What is preferred way to contact with
> you?
If the question is related to this review request, you can write it on
this bug. Otherwise feel free to mail to me.

Might I add that "For Easy Rule Making" as a Summary really doesn't say anything about what this package does.  I would suggest including "firewall rule manager" or at least something about firewalls in the Summary.

I got answer from Max Kellerman, one of upstream ferm developer:

> What is more true GPLv2 or "GPLv2 or any later version" (GPLv2+ in  
> > Fedora terminology)??
...
The home page is only 99.9% accurate, to keep it as short as possible
and readable - you know, nobody reads text in a web browser,
especially if it's longer than really needed  ;-) 

The only "real" licensing statement is the one in the source code,
which should be "GPLv2+" for you.

So, I'm change Licence to GPLv2+

http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm.spec
http://hubbitus.net.ru/rpm/Fedora9/ferm/ferm-2.0.2-3.fc9.src.rpm

Now I reading and working on importing this package into Fedora 8/9.

@Jason Tibbitts

I have follow guidelines and got summary from upstream.
May be "For Easy firewall Rule Making"? Furthermore it is abbrivation as I understood.

New Package CVS Request
=======================
Package Name: ferm
Short Description: For Easy Rule Making
Owners: hubbitus
Branches: F-8 F-9
InitialCC:

cvs done.

Guake 0.3.1 released will you update it?

Sorry, last post was erroneous.

ferm-2.0.2-3.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/ferm-2.0.2-3.fc9

ferm-2.0.2-3.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/ferm-2.0.2-3.fc8

Now closing.

ferm-2.0.2-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

ferm-2.0.2-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

ferm-2.0.2-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: ferm
New Branches: EL-5
Owners: hubbitus

You need not reopen the bug for CVS request.

CVS done.  And yes, we don't care if the ticket is open; we just look for the status of the fedora-cvs flag.

Thank you. And ok, I'll known in next times.

ferm-2.0.7-8.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/ferm-2.0.7-8.el5

ferm-2.0.7-8.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.