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=233070
Summary: gnome-web-photo: HTML pages thumbnailer Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: bnocera@redhat.com QAContact: fedora-package-review@redhat.com
gnome-web-photo contains a thumbnailer that will be used by GNOME applications, including the file manager, to generate screenshots of web pages.
Packages and spec at: http://www.gnome.org/~hadess/gnome-web-photo/gnome-web-photo-0.3-1.src.rpm http://www.gnome.org/~hadess/gnome-web-photo/gnome-web-photo.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233070
------- Additional Comments From bnocera@redhat.com 2007-03-20 08:38 EST ------- Note that the .spec file changes the filename of "thumbnailer.schemas" to "gnome-web-photo.schemas". This should probably be done upstream as well.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233070
peter@thecodergeek.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |peter@thecodergeek.com
------- Additional Comments From peter@thecodergeek.com 2007-03-21 01:43 EST ------- Not a full review, but some comments from looking over the spec file:
* For your URL tag, Is there a specific reason for choosing the ftp.acc.umu.se mirror rather than the round-robin ftp.gnome.org hostname?
* Your Source0 tag should be a fully-qualified URL to the tarball; or if it's a Fedora-hosted project (wherein this packaging *is* the upstream source), you should add a comment to note it as such.
* Does it really need the gettext development environment, or does it just use the gettext utilities for its translation merging while building? If it just uses the utilities, then please change the "BuildRequires: gettext-devel" to simply "BuildRequires: gettext"
* You don't need a build-time dependency on libpng-devel, since it's a dependency of gtk2-devel. Similarly, gnome-vfs2-devel has a depenency on GConf2-devel, so you should remove libpng-devel and GConf2-devel from your BuildRequires since they are duplicated entries.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Product|Fedora Extras |Fedora
------- Additional Comments From bnocera@redhat.com 2007-09-05 09:57 EST ------- (In reply to comment #2)
Not a full review, but some comments from looking over the spec file:
- For your URL tag, Is there a specific reason for choosing the ftp.acc.umu.se
mirror rather than the round-robin ftp.gnome.org hostname?
Fixed
- Your Source0 tag should be a fully-qualified URL to the tarball; or if it's a
Fedora-hosted project (wherein this packaging *is* the upstream source), you should add a comment to note it as such.
Fixed.
- Does it really need the gettext development environment, or does it just use
the gettext utilities for its translation merging while building? If it just uses the utilities, then please change the "BuildRequires: gettext-devel" to simply "BuildRequires: gettext"
gettext should be enough.
- You don't need a build-time dependency on libpng-devel, since it's a
dependency of gtk2-devel. Similarly, gnome-vfs2-devel has a depenency on GConf2-devel, so you should remove libpng-devel and GConf2-devel from your BuildRequires since they are duplicated entries.
Done.
Fixed packages at: http://www.gnome.org/~hadess/gnome-web-photo/gnome-web-photo-0.3-2.src.rpm http://www.gnome.org/~hadess/gnome-web-photo/gnome-web-photo.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
------- Additional Comments From mclasen@redhat.com 2007-09-06 01:50 EST ------- formal review:
rpmlint: gnome-web-photo.i386: W: non-conffile-in-etc /etc/gconf/schemas/gnome-web-photo.schemas
this follows existing practise, thus is ok
gnome-web-photo.i386: W: incoherent-version-in-changelog 0.3.2 0.3-2.fc8
should be fixed
gnome-web-photo.i386: W: invalid-license GPL
must be fixed (see below)
gnome-web-photo.src: W: mixed-use-of-spaces-and-tabs (spaces: line 38, tab: line 38)
unimportant
package name: ok spec file name: ok packaging guidelines: - I don't think there is any reason to use %{__rm} - You need a %postun for gconf schemas, I believe - The preferred form of the requires is perl(XML::Parser) license: unclear, COPYING is GPLv2, but the sources all say LGPLv2+, should be clarified upstream license field: should be updated to match the result of aforementioned clarification license file: ok spec file language: ok spec file legibility: ok upstream sources: ok build: fails, see below excludearch: n/a build reqs: misses libjpeg-devel %find_lang: ok shared library symlinks: n/a relocatable: n/a directory ownership: ok duplicate files: ok file permissions: ok %clean: ok macro use: ok permissible content: ok large docs: n/a %doc content: ok header files: n/a static libs: n/a pc files: n/a shared libs: n/a devel package: n/a libtool archives: n/a gui apps: ok directory ownership: ok %install: ok utf8 filenames: ok
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
------- Additional Comments From bnocera@redhat.com 2007-09-06 10:12 EST ------- (In reply to comment #4)
formal review:
<snip>
gnome-web-photo.i386: W: incoherent-version-in-changelog 0.3.2 0.3-2.fc8
should be fixed
Typo, fixed.
gnome-web-photo.i386: W: invalid-license GPL
must be fixed (see below)
LGPLv2.1+ as per COPYING.README. I believe GPL refers to some of the build tools/scripts.
gnome-web-photo.src: W: mixed-use-of-spaces-and-tabs (spaces: line 38, tab:
line 38)
unimportant
Fixed anyway.
package name: ok spec file name: ok packaging guidelines:
- I don't think there is any reason to use %{__rm}
Fixed.
- You need a %postun for gconf schemas, I believe
The schemas is removed in %preun, nothing to do in %postun, as the schemas's already gone.
- The preferred form of the requires is perl(XML::Parser)
license: unclear, COPYING is GPLv2, but the sources all say LGPLv2+, should be clarified upstream
See above.
license field: should be updated to match the result of aforementioned clarification
Done.
license file: ok
I put just the COPYING.README, should I also package the COPYING.LGPL?
build reqs: misses libjpeg-devel
Fixed.
http://www.gnome.org/~hadess/gnome-web-photo/gnome-web-photo-0.3-3.src.rpm http://www.gnome.org/~hadess/gnome-web-photo/gnome-web-photo.spec
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
mclasen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review+
------- Additional Comments From mclasen@redhat.com 2007-09-06 12:18 EST ------- For the license: yes, please include the license file that applies to the source, ie COPYING.LGPL, if it is in the upstream tarball.
The rest looks fine now. Approved.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
bnocera@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
------- Additional Comments From bnocera@redhat.com 2007-09-08 06:51 EST ------- New Package CVS Request ======================= Package Name: gnome-web-photo Short Description: HTML pages thumbnailer Owners: hadess Branches: InitialCC: Cvsextras Commits: yes
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
------- Additional Comments From kevin@tummy.com 2007-09-09 18:50 EST -------
From the Licensing page, the tag here should be "LGPLv2+" (Thats used for both 2
and 2.1).
cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: gnome-web-photo: HTML pages thumbnailer
https://bugzilla.redhat.com/show_bug.cgi?id=233070
bnocera@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution| |RAWHIDE
------- Additional Comments From bnocera@redhat.com 2007-09-10 11:57 EST ------- (In reply to comment #8)
From the Licensing page, the tag here should be "LGPLv2+" (Thats used for both 2 and 2.1).
Fixed, thanks!
Uploaded as gnome-web-photo-0.3-4.fc8
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla