[Bug 198928] Review Request: lsscsi

[bugzilla at redhat.com]

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


cweyl at alumni.drew.edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cweyl at alumni.drew.edu




------- Additional Comments From cweyl at alumni.drew.edu  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.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.