[Bug 199780] Review Request: dstat

[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: dstat


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





------- Additional Comments From toshio at tiki-lounge.com  2006-07-25 15:47 EST -------
MD5sums:
72917aa5eed385464d70ec731bd6d2b1  dstat-0.6.3-2.src.rpm
a2df5d7fecc0115f8eef84141a068e86  dstat-0.6.3.tar.bz2

Blocker:
* Your patch adds #!/usr/bin/python to the modules files.  Since the modules are
 not intended to be run from the command line, this is improper behaviour.  You
 should make the files non-executable instead.
* It's customary to put the defattr line first in the files section.  I believe
  that rpm uses the defattr line even on lines that preceed it in the file
  listings but I don't know if that's documented behaviour.  If that behaviour
  ever changes, then you could suddenly have files with incorrect ownership and
  permissions.
* Remove the execute permissions from the scripts in the examples directory.
* Package needs to own the dstat directory like this:
  %dir %{_datadir}/dstat

Cosmetic:
* It looks like you're patching out dos line endings for html files.  This is
  okay if you're going to submit the change upstream.  As a general rule (for
  instance, if upstream were to not accept the change and you had to continue
  to carry it around in the package) it's better to use sed or dos2unix to fix
  this.  Otherwise your patch becomes filled with whole files where the only
  difference is the EOL character.
* Some people still use the default rpm defined topdir, sourcedir, etc.  When
  this is so, installing the source rpm places the files in the same directory
  as a multitude of other rpms.  So your patch: patch-to-clean-for-fedora.patch
  should really have a less generic name.  Something like dstat-eol.patch or
  dstat-eol-cleanup.pach.
* I'd remove the commented out code as it doesn't add any value to the spec.
* You don't need to Require: python; this is being picked up automatically.

Good:
* Source matches upstream.
* Package Name follows the Naming Guidelines.
* Spec file follows the %{name}.spec format.
* License is GPL, matches the license field, and is included in the rpm.
* Package built on x86_64 as a noarch package.
* All BuildRequires not listed in the exceptions have been satisfied.
* No locale files, so no need to use %find_lang.
* No libraries or pkgconfig files.
* Package is not relocatable.
* No duplicate files.
* Package has a proper %clean section.
* Package uses proper macros from Packaging Guidelines.
* Code, not content.
* No large doc files.
* Doc files do not affect the runtime behaviour.
* Not a GUI application.
* Does not own directories that belong to another package.
* No scriptlets.
* Package built in mock.

After you fix the listed issues, I can run a last rpmlint check and make sure
the program runs and then APPROVE the package.

Since you need a sponsor, I need to know that you understand the guidelines
before SPONSORing you.  This package was a good indication of understanding on
your part.  If you do a review of someone else's package that continues to
demonstrate your knowledge of the Fedora Guidelines in particular and rpm
packaging in general, I'll sponsor you as well.

-- 
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.