[Bug 198928] New: Review Request: <main package name here>

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

Summary: Review Request: <main package name here>
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]

Spec URL: http://people.redhat.com/coldwell/lsscsi.spec
SRPM URL: http://people.redhat.com/coldwell/lsscsi-0.17-2.src.rpm
Description: Uses information provided by the sysfs pseudo file system in Linux kernel 2.6 series to list SCSI devices or all SCSI hosts. Includes a "classic" option to mimic the output of "cat /proc/scsi/scsi" that has been widely used prior to the lk 2.6 series.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: <main       |Review Request: lsscsi
                   |package name here>          |

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

------- Additional Comments From [e-mail]  2006-07-14 21:53 EST -------
Are you sponsored into extras yet (as I'm not seeing you on the cvsextras page).
 If not, you should mark this bug as blocking FE-NEEDSPONSOR.

A couple thoughts on the spec, off the top of my head:

* The macros you have defined at the top are rather redundant, as you use them
to then populate the Name:, Version:, Release: tags, all of which (when
populated) define macros of the same names you're using containing the exact
same information.

* You're using both, e.g., $RPM_BUILD_ROOT and %{buildroot}.  Either or is fine,
but the packaging guidelines require us to be consistent within a specfile (this
is a "MUST").

* In build, why not use "%configure"?  Similarly, in install, why not
"%makeinstall"?  Both of those macros should take care of passing the
information you're doing manually.  (Also, it would quiet rpmlint about "E:
lsscsi configure-without-libdir-spec".)

* There may be comments on the test before "rm -rf ..." in %clean.  I think it's
ok, as it will always evaluate to true in the buildsys, but it could be nixed
for brevity's sake.

That being said, I've built and installed this package on my system.  Works
nicely for me, even against my SATA drives.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-15 09:41 EST -------
(In reply to comment #1)
> In build, why not use "%configure"?  Similarly, in install, why not
>
> "%makeinstall"?

%makeinstall is deprecated. See the "Macros" section of the Packaging Guidelines.

http://fedoraproject.org/wiki/Packaging/Guidelines
> There may be comments on the test before "rm -rf ..." in %clean.  I think it's
>
> ok, as it will always evaluate to true in the buildsys, but it could be nixed
> for brevity's sake.

Not only that but the tests are completely redundant since a buildroot is
explicitly specified in the spec file.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-17 09:46 EST -------
(In reply to comment #1)
> Are you sponsored into extras yet (as I'm not seeing you on the cvsextras page).
>  If not, you should mark this bug as blocking FE-NEEDSPONSOR.
>
> A couple thoughts on the spec, off the top of my head:
> The macros you have defined at the top are rather redundant, as you use them
>
> to then populate the Name:, Version:, Release: tags, all of which (when
> populated) define macros of the same names you're using containing the exact
> same information.
> You're using both, e.g., $RPM_BUILD_ROOT and %{buildroot}.  Either or is fine,
>
> but the packaging guidelines require us to be consistent within a specfile (this
> is a "MUST").
> In build, why not use "%configure"?  Similarly, in install, why not
>
> "%makeinstall"?  Both of those macros should take care of passing the
> information you're doing manually.  (Also, it would quiet rpmlint about "E:
> lsscsi configure-without-libdir-spec".)
> There may be comments on the test before "rm -rf ..." in %clean.  I think it's
>
> ok, as it will always evaluate to true in the buildsys, but it could be nixed
> for brevity's sake.
>
> That being said, I've built and installed this package on my system.  Works
> nicely for me, even against my SATA drives.

Good points, thanks.  Just to clarify, the spec file comes from the project
homepage; I just touched it up to silence some rpmlint complaints.  However,
it looks like it wasn't enough, so I'll incorporate your recommendations.

lsscsi is a very desirable addition since /proc/scsi/scsi runs out of memory
if there are a large number of LUNs (>512, I think), and any kernel patch
submitted to fix this problem is rejected (/proc/scsi/scsi is deprecated)
and the submitter is referred to lsscsi:

http://marc.theaimsgroup.com/?l=linux-kernel&m=111287017419578&w=2

Chip

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-17 10:33 EST -------
I updated the spec file and src.rpm to incorporate your recommendations; they're
still in the same place:

http://people.redhat.com/coldwell/lsscsi.spec
http://people.redhat.com/coldwell/lsscsi-0.17-2.src.rpm

(don't know if I should have bumped the release number or not).

Chip

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |177841
              nThis|                            |

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-20 02:30 EST -------
Spec file looks much cleaner, and builds in mock (5&devel/x86_64).

And yes, always bump the release number :)

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail] |[e-mail]
OtherBugsDependingO|163776, 177841              |163779
              nThis|                            |

------- Additional Comments From [e-mail]  2006-07-20 22:13 EST -------
Chip, I'll sponsor you, now that Chris has done all of the work.

rpmnlint has one complaint about the spec:
  W: lsscsi mixed-use-of-spaces-and-tabs
cat -T shows this coming from spaces after the colons in Summary:, Name:,
Version:, and Release:.  Terribly minor, I know, but there's no reason not to
fix it.  (You only see this if you run rpmlint against the src.rpm.)

I guess it's pretty pointless for a package with one source file, but generally
folks with multiprocessor machines appreciate it when you add %{?_smp_mflags} to
your make line to build things in parallel.  As long as it doesn't break the
build, of course.  (And if it does break the build, add a comment to indicate
that fact.)

The only other thing is the oddity of including the author information at the
end of %description.  Then again, it would be somewhat uncouth to grab his
specfile and then cut his name out of it, so I'm not sure what to do.  I'll
leave it up to you.

With only the space/tab thing to fix, I'll go ahead and approve this and you can
fix it when you check in.

Review:
* source files match upstream:
   c05c1cc6e6c425d86bf41e2ab0f09172  lsscsi-0.17.tgz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* Compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
X rpmlint is silent (minor tabs/spaces thing)
* debuginfo package looks complete.
* final provides and requires are sane:
   lsscsi = 0.17-2
  =
   libc.so.6()(64bit)
* %check is not present; no test suite upstream.
* no shared libraries are present.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.  (Technically the
documentation is larger then the executable, but we're talking about 25K total
here.)
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.

APPROVED

Go ahead and request cvsextras and fedorabugs access and I'll approve you.  See
http://fedoraproject.org/wiki/Extras/Contributors#GetAFedoraAccount for details.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-24 10:42 EST -------
OK, I replaced the tabs with spaces and added the SMP flags to the %make macro.
The new spec/srpm are in the same place:

http://people.redhat.com/coldwell/lsscsi.spec
http://people.redhat.com/coldwell/lsscsi-0.17-2.src.rpm

I also did the Fedora rigamarole -- username 'coldwell'.

Thanks,

Chip

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-24 11:27 EST -------
You should be all set up now; let me know if you have any trouble getting things
imported and built.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-24 11:50 EST -------
http://fedoraproject.org/wiki/Extras/NewPackageProcess

step 9 is failing; the owners CVS module doesn't seem to exist

CVSROOT is :gserver:cvs.devel.redhat.com:/cvs/dist

Chip

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-24 13:02 EST -------
That CVSROOT is wrong for FE.  See http://fedoraproject.org/wiki/Extras/UsingCvsFaq

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-25 11:05 EST -------
I was using CVS with the Fedora CVSROOT yesterday, but today I'm locked out:

$ export CVSROOT=:ext:[e-mail]:/cvs/extras
$ export CVS_RSH=ssh
$ cvs co lsscsi
For more information on using the Fedora source code repositories, 
please visit http://fedoraproject.org/wiki/UsingCvs
Permission denied (publickey,keyboard-interactive).
cvs [checkout aborted]: end of file from server (consult above messages if any)

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-25 11:12 EST -------
Dumb question, I know, but can I assume that you're not actually using
"USERNAME" there?

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-25 11:14 EST -------
(In reply to comment #12)
> Dumb question, I know, but can I assume that you're not actually using
> "USERNAME" there?

Ooops.  The hazards of copy/paste from the FAQ.  Thanks.

Chip

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-25 11:16 EST -------
(In reply to comment #13)
> (In reply to comment #12)
> > Dumb question, I know, but can I assume that you're not actually using
> > "USERNAME" there?
>
> Ooops.  The hazards of copy/paste from the FAQ.  Thanks.

I take it back:

$ cvs co lsscsi
For more information on using the Fedora source code repositories, 
please visit http://fedoraproject.org/wiki/UsingCvs
Permission denied (publickey,keyboard-interactive).
cvs [checkout aborted]: end of file from server (consult above messages if any)
$ set | grep CVS
CVSROOT=:ext:[e-mail]:/cvs/extras
CVS_RSH=ssh

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

------- Additional Comments From [e-mail]  2006-07-25 13:14 EST -------
I don't know, honestly.  Perhaps the public key on that machine doesn't match
what you sent?

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

------- Additional Comments From [e-mail]  2006-07-26 10:53 EST -------
Date: Wed, 26 Jul 2006 10:55:06 -0400 (EDT)
From: [e-mail]
To: [e-mail]
Subject: Build Result: 13160 - lsscsi on fedora-development-extras

13160 (lsscsi): Build on target fedora-development-extras succeeded.
     Build logs may be found at
http://buildsys.fedoraproject.org/logs/fedora-development-extras/13160-lsscs
i-0.17-2/

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|NEXTRELEASE                 |RAWHIDE

------- Additional Comments From [e-mail]  2006-07-26 13:48 EST -------
http://brewweb.devel.redhat.com/brew/taskinfo?taskID=135258

emacs-21.4-15 built.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|RAWHIDE                     |NEXTRELEASE

------- Additional Comments From [e-mail]  2006-07-26 13:49 EST -------
Ooops, wrong bug.

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |194453
              nThis|                            |

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: lsscsi

https://bugzilla.redhat.com/show_bug.cgi?id=198928

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |medium
           Priority|normal                      |medium
            Product|Fedora Extras               |Fedora
            Version|devel                       |rawhide

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=198928

Dan Horák  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]
               Flag|                            |fedora-cvs?

--- Comment #19 from Dan Horák   2009-11-25 14:55:19 EDT ---
Package Change Request
======================
Package Name: lsscsi
New Branches: EL-4 EL-5
Owners: sharkcz

[Bug 198928] Review Request: lsscsi

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=198928

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #20 from Kevin Fenzi   2009-11-27 00:57:34 EDT ---
cvs done.