Builds successfully in mock on Fedora 11 with i386/x86_64 architectures.

Builds successfully in mock on Fedora 13 with i386/x86_64 architectures.

* Sat Mar 13 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-2
- Fix the license
- Fix the summary
- Replace generally useful macros by regular commands


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-2.fc12.src.rpm


The srpm builds successfully by the mock on Fedora 11, 12, 13 with i386/x86_64
architectures.

The rpmlint output is clean.

Since the nmbscan script relies on ping, you should add iputils to the requirements. 

arp is provided by package net-tools (missing in spec)
nslookup is provided from bind-utils (missing in spec)

I could do an inofficial review, since I'm not sponsored yet.

Fixed

Spec diff:
@@ -1,6 +1,6 @@
 Name:           nmbscan
 Version:        1.2.5
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        NMB/SMB network scanner

 Group:          Applications/Internet
@@ -10,6 +10,9 @@
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch

+Requires:       bind-utils
+Requires:       iputils
+Requires:       net-tools
 Requires:       samba-client


@@ -49,6 +52,9 @@


 %changelog
+* Fri Mar 19 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-3
+- Add Requires: bind-utils, iputils, net-tools


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-3.fc12.src.rpm

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Package is named according to the Package Naming Guidelines.
[x]  Spec file name must match the base package %{name}, in the format %{name}.spec.
[ ]  Package meets the Packaging Guidelines.
[x]  Package successfully compiles and builds into binary rpms on at least one supported architecture.
Tested on:
[x]  Rpmlint output:
[mrunge@sofja rpmbuild]$ rpmlint nmbscan*
nmbscan.src: W: spelling-error %description -l en_US hostname -> host name, host-name, hostage
nmbscan.src: W: spelling-error %description -l en_US ethernet -> Ethernet, ether net, ether-net
nmbscan.src: W: spelling-error %description -l en_US username -> user name, user-name, whatshername
Error checking signature of nmbscan-1.2.5-3.fc12.src.rpm: nmbscan-1.2.5-3.fc12.src.rpm: (SHA1) DSA sha1 md5 (GPG) NOT OK (MISSING KEYS:GPG#d8ab0cec) 
1 packages and 1 specfiles checked; 0 errors, 3 warnings.

I guess, thats ok.
[?]  Package is not relocatable.
[x]  Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
[x]  Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
[x]  License field in the package spec file matches the actual license.
License type: GPLv2+
[x]  If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.
MD5SUM this package    : 7cbfd9c7ea817e67525006e78fb5d32e
MD5SUM upstream package: 7cbfd9c7ea817e67525006e78fb5d32e
[x]  Package is not known to require ExcludeArch, OR:
Arches excluded:
Why:
[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
[-]  The spec file handles locales properly.
[-]  ldconfig called in %post and %postun if required.
[-]  Package must own all directories that it creates.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  Permissions on files are set properly.
[x]  Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[x]  Package consistently uses macros.
[x]  Package contains code, or permissable content.
[-]  Large documentation files are in a -doc subpackage, if required.
[x]  Package uses nothing in %doc for runtime.
[-]  Header files in -devel subpackage, if present.
[-]  Static libraries in -devel subpackage, if present.
[-]  Package requires pkgconfig, if .pc files are present.
[-]  Development .so files in -devel subpackage, if present.
[-]  Fully versioned dependency in subpackages, if present.
[x]  Package does not contain any libtool archives (.la).
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI application.
[x]  Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
[x]  Latest version is packaged.
[?]  Package does not include license text files separate from upstream.
[-]  Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
[x]  Reviewer should test that the package builds in mock.
Tested on: fedora-12-i386
[x]  Package should compile and build into binary rpms on all supported architectures.
Tested on: noarch
[x]  Package functions as described.
[-]  Scriptlets must be sane, if used.
[-]  The placement of pkgconfig(.pc) files are correct.
[-]  File based requires are sane.


I would approve it, but since I'm not approved yet, this review is just informal.

Hi Nikolay,

according to the copyright notice included in the shell script, the license of nmbscan is GPLv2, not GPLv2+ because the addition "or (at your option) any later version" is missing.

I also suggest to use the macro %{name} in Source0 and %file:
Source0: http://nmbscan.gbarbier.org/down/%{name}-%{version}.tar.gz
%{_bindir}/%{name}

Hi Martin

Spec diff:
-Release:        3%{?dist}
+Release:        4%{?dist}
...
-License:        GPLv2+
+License:        GPLv2
...
-Source0:        http://nmbscan.gbarbier.org/down/nmbscan-%{version}.tar.gz
+Source0:        http://nmbscan.gbarbier.org/down/%{name}-%{version}.tar.gz
...
-%{_bindir}/nmbscan
+%{_bindir}/%{name}
...
 %changelog
+* Sat Apr 17 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-4
+- Fix the license and Source0


Spec URL: http://repo.lystor.org.ua/fedora/12/SPECS/nmbscan.spec
SRPM URL:
http://repo.lystor.org.ua/fedora/12/SRPMS/nmbscan-1.2.5-4.fc12.src.rpm

Since nobody else seems to be interested in this package, I take the review. :)
Matthias already did a good job with his informal review, and there are just two cosmetic things left I would address:

- Add a comment to the %build section telling that there's nothing to build.

- As suggested by the spell checker, capitalize "Ethernet" in the %description. The other spelling errors can be ignored.


$ rpmlint /var/lib/mock/fedora-12-x86_64/result/*.rpm
nmbscan.noarch: W: spelling-error %description -l en_US hostname -> host name, host-name, hostage
nmbscan.noarch: W: spelling-error %description -l en_US ethernet -> Ethernet, ether net, ether-net
nmbscan.noarch: W: spelling-error %description -l en_US username -> user name, user-name, whatshername
nmbscan.src: W: spelling-error %description -l en_US hostname -> host name, host-name, hostage
nmbscan.src: W: spelling-error %description -l en_US ethernet -> Ethernet, ether net, ether-net
nmbscan.src: W: spelling-error %description -l en_US username -> user name, user-name, whatshername
2 packages and 0 specfiles checked; 0 errors, 6 warnings.

---------------------------------
keys used in following checklist:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - GPLv2 according to text in shell script

[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum nmbscan-1.2.5.tar.gz*
    7cbfd9c7ea817e67525006e78fb5d32e  nmbscan-1.2.5.tar.gz
    7cbfd9c7ea817e67525006e78fb5d32e  nmbscan-1.2.5.tar.gz.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
    - shell script doesn't need to be built
    - package builds in koji:
      http://koji.fedoraproject.org/koji/taskinfo?taskID=2183527

[.] MUST: If the package does not successfully compile ...
[+] MUST: All build dependencies must be listed in BuildRequires.
    - none required

[.] MUST: The spec file MUST handle locales properly.
    - no locales
  
[.] MUST: Packages storing shared library files must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix ...
[.] MUST: devel packages must require the base package 
[+] MUST: Packages must NOT contain any .la libtool archives
[.] MUST: Packages containing GUI applications ...
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[+] SHOULD: The reviewer should test that the package builds in mock.
    - builds in mock

[.] SHOULD: The package should compile and build into binary rpms on all supported architectures.
    - noarch package

[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) should be placed in a -devel pkg.

Nikolay, are you still interested in this package? If so, please address the small remaining issues so that we can finish the review.

Hello,
as there is long time no update from Nikolay I volunteer to finish those cosmetic changes and co-maintain the package.

Here is updated package with cosmetic changes and manual page :
http://rebus.fedorapeople.org/13/SPECS/nmbscan.spec
http://rebus.fedorapeople.org/13/SRPMS/nmbscan-1.2.5-5.fc13.src.rpm

Output from rpmlint:
$ rpmlint nmbscan-1.2.5-5.fc13.src.rpm nmbscan-1.2.5-5.fc13.noarch.rpm 
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Best regards
Michal Ambroz

If Nikolay is busy and/or cannot finish this submission process, this bug should be closed and a new one should be submitted by the new maintainer.

(In reply to comment #12)
> this bug should be closed and a new one should be submitted
> by the new maintainer.    
Hello Manuel, 
I beg to differ - I believe it is better to have it on one place and not split the effort. There is nothing wrong if package has more than one maintainer. In fact it is encouraged to be like that to ensure that package gets proper maintenance in future. 

In Package Review Process (http://fedoraproject.org/wiki/Package_Review_Process) there are defined roles, but it is not said that there can't be more than one maintainer.

In fact in the CVS Admin request you can see even in the example that there could be more package maintainers from the very beginning. 
(http://fedoraproject.org/wiki/Package_SCM_admin_requests#New_Packages)

Manuel - please could you support your claim by some link to some procedure/guideline, which says it should be closed and new reopened? 
I have not found it codified anywhete. If it is just your gut feeling, please reconsider the position.

Best regards
Michal Ambroz

Manuel is correct, that is normal practice. Even though it can be considered normal for a package to have *co*maintainers, there is always a *primary* maintainer who is considered responsible.

If Nikolay is not any more willing to tend to this review, you (Michal) must open up a new review request and mark this one as its duplicate.

Michal,

I think, it's great, if you volunteer and take over this package. Since I can
not see, what information this page here provides you, which a new request can
not provide.

The bug reporter is the person maintaining the package. There is a way to take
over a package, but only, if the package is approved. If I understand it
correctly, co-maintainer get scm access (later on).

What is the problem with the solution, Manuel suggested? I'm sure, someone
(Martin, Manuel(?), or me) will review your package.
Matthias

> Nikolay, are you still interested in this package? If so, please address the
small remaining issues so that we can finish the review.

Hi
Yes. I am still interested in this package and I will finish the nmbscan review process as soon as I can.

Also I allow Michal Ambroz to co-maintain the package.

Spec diff:
--- nmbscan.spec.orig	2010-04-17 17:26:19.000000000 +0300
+++ nmbscan.spec	2010-08-05 10:34:04.730173241 +0300
@@ -1,12 +1,13 @@
 Name:           nmbscan
 Version:        1.2.5
-Release:        4%{?dist}
+Release:        5%{?dist}
 Summary:        NMB/SMB network scanner
 
 Group:          Applications/Internet
 License:        GPLv2
-URL:            http://nmbscan.gbarbier.org/
-Source0:        http://nmbscan.gbarbier.org/down/%{name}-%{version}.tar.gz
+URL:            http://nmbscan.g76r.eu/
+Source0:        http://nmbscan.g76r.eu/down/%{name}-%{version}.tar.gz
+Source1:        %{name}.1
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 
@@ -20,9 +21,9 @@
 Scans a SMB shares network, using NMB and SMB protocols. Useful to acquire
 an information on a local area network (security audit, etc.)
 
-Matches the information such as NMB/SMB/Windows hostname, IP address, IP
-hostname, ethernet MAC address, Windows username, NMB/SMB/Windows domain name
-and master browser.
+Matches the information such as NMB/SMB/Windows host name, IP address,
+IP host name, Ethernet MAC address, Windows user name,
+NMB/SMB/Windows domain name and master browser.
 
 Can discover all NMB/SMB/Windows hosts on a local area network thanks to 
 hosts lists maintained by master browsers.
@@ -33,12 +34,15 @@
 
 
 %build
+# Nothing to build
 
 
 %install
 rm -rf %{buildroot}
 install -d %{buildroot}%{_bindir}
-install -p -m 755 nmbscan %{buildroot}%{_bindir}/
+install -p -m 0755 nmbscan %{buildroot}%{_bindir}/
+install -d %{buildroot}%{_mandir}/man1
+install -p -m 0644 %{SOURCE1} %{buildroot}%{_mandir}/man1/
 
 
 %clean
@@ -49,9 +53,15 @@
 %defattr(-,root,root,-)
 %doc Documentation/HOWTO_contribute.txt Documentation/gplv2.txt
 %{_bindir}/%{name}
+%{_mandir}/man1/%{name}.1*
 
 
 %changelog
+* Wed Aug 04 2010 Nikolay Ulyanitsky <lystor AT lystor.org.ua> - 1.2.5-5
+- Fixed the project URL
+- Added a man page (thx to Michal Ambroz)
+- Cosmetic changes


$ rpmlint nmbscan-1.2.5-5.fc13.noarch.rpm nmbscan-1.2.5-5.fc13.src.rpm 
2 packages and 0 specfiles checked; 0 errors, 0 warnings.


Spec URL: http://repo.lystor.org.ua/fedora/13/SPECS/nmbscan.spec
SRPM URL: http://repo.lystor.org.ua/fedora/13/SRPMS/nmbscan-1.2.5-5.fc13.src.rpm

Nikolay, thanks for your feedback. The package is ready now.

----------------
Package APPROVED
----------------

New Package SCM Request
=======================
Package Name: nmbscan
Short Description: NMB/SMB network scanner
Owners: lystor rebus
Branches: f13 f14
InitialCC:

Git done (by process-git-requests).

nmbscan-1.2.5-5.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/nmbscan-1.2.5-5.fc13

nmbscan-1.2.5-5.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/nmbscan-1.2.5-5.fc14

nmbscan-1.2.5-5.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

nmbscan-1.2.5-5.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

nmbscan-1.2.6-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/nmbscan-1.2.6-1.fc13

nmbscan-1.2.6-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/nmbscan-1.2.6-1.fc14

nmbscan-1.2.6-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

nmbscan-1.2.6-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: nmbscan
New Branches: el6 epel7
Owners: fab rebus
InitialCC:

Git done (by process-git-requests).