[Bug 636819] Review Request: gnome-exe-thumbnailer - gnome thumbnailer for exe files

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


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

Hans de Goede <hdegoede at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-review?

--- Comment #33 from Hans de Goede <hdegoede at redhat.com> 2011-04-11 16:08:22 EDT ---
And here is the long promised full review:

Good:
=====
- rpmlint checks return:
 gnome-exe-thumbnailer.src: W: spelling-error %description -l en_US py -> pt,
p, y
 gnome-exe-thumbnailer.noarch: W: spelling-error %description -l en_US py ->
pt, p, y
 gnome-exe-thumbnailer.noarch: W: no-manual-page-for-binary
gnome-exe-thumbnailer.sh
 These can be ignored
- package meets naming guidelines
- package meets packaging guidelines
- license (LGPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Needs work:
===========
- rpmlint says:
 gnome-exe-thumbnailer.noarch: W: empty-%pre
 gnome-exe-thumbnailer.noarch: W: empty-%post
 gnome-exe-thumbnailer.noarch: W: empty-%preun
 This can simply be fixed by moving the "%if 0%{?fedora} < 15"
 above the %pre / %post / %preun
- %pre is in a weird place in the specfile, normally all installation
  scripts (be it pre or post) are put between  the %clean and the
  %files section in the file
- %post / %preun are in a bit of a weird place either, not as weird as
  %pre thoug. But normally they are after %clean instead of before it
- Please put a blank line in the changelog before each data-name-version line
  (except for the first). And prefix / itemize items below the
  data-name-version line with "- ". IE change:
   * Sun Sep 26 2010 Elad Alfassa <el.il at doom.co.il> - 0.6-3
   Fix typo.
   * Sun Sep 26 2010 Elad Alfassa <el.il at doom.co.il> - 0.6-2
   Mark schemas file as config file
   * Sun Sep 26 2010 Elad Alfassa <el.il at doom.co.il> - 0.6-1
   More spec file fixes
   * Thu Sep 23 2010 Elad Alfassa <elad at macron.co.il> - 0.6-0
   Version update, some general spec file fixes.
   * Sun Jul 25 2010 Elad Alfassa <elad at macron.co.il> - 0.3-1
   initial build
  to:
   * Sun Sep 26 2010 Elad Alfassa <el.il at doom.co.il> - 0.6-3
   - Fix typo.

   * Sun Sep 26 2010 Elad Alfassa <el.il at doom.co.il> - 0.6-2
   - Mark schemas file as config file

   * Sun Sep 26 2010 Elad Alfassa <el.il at doom.co.il> - 0.6-1
   - More spec file fixes

   * Thu Sep 23 2010 Elad Alfassa <elad at macron.co.il> - 0.6-0
   - Version update, some general spec file fixes.

   * Sun Jul 25 2010 Elad Alfassa <elad at macron.co.il> - 0.3-1
   -initial build

- Release should start at 1 after a version bump, not 0. This is not something
  to fix retroactively but to keep in mind for the next time upstream
  releases a new version.


All the need work items are rather small things, so all in all this looks good.
Based on your informal reviews as well as your withdrawn submission for 2 more
packages I'm ready to sponsor you as soon as a new gnome-exe-thumbnailer fixing
the mentioned (small) issues are fixed.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.