Bug 703719 - Review Request: spice-xpi - mozilla extension for spice client
Review Request: spice-xpi - mozilla extension for spice client
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
:
: 703720 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-11 03:55 EDT by Peter Hatina
Modified: 2016-05-31 21:30 EDT (History)
4 users (show)

See Also:
Fixed In Version: spice-xpi-2.5-2.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-06-21 13:11:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Peter Hatina 2011-05-11 03:55:58 EDT
Spec URL: http://www.stud.fit.vutbr.cz/~xhatin01/spice-xpi/spice-xpi.spec
SRPM URL: http://www.stud.fit.vutbr.cz/~xhatin01/spice-xpi/spice-xpi-2.5-1.fc15.src.rpm
Description: 

Hi, I would like to have this new package reviewed.

Spice-xpi is NPAPI based web browser extension, which allows to run spice client from browser.
Comment 1 Elad Alfassa 2011-05-11 04:19:46 EDT
*** Bug 703720 has been marked as a duplicate of this bug. ***
Comment 2 Hans de Goede 2011-05-11 04:45:13 EDT
I'll review this and if all goes well sponsor Peter, assigning to me.
Comment 3 Hans de Goede 2011-06-07 09:07:11 EDT
Hi,

Full review done:

Good:
=====
- package meets naming guidelines
- spec file legible, in am. english
- package compiles on devel (x86)
- no missing BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file

Needs work:
===========
- rpmlint checks return:
[hans@shalem ~]$ rpmlint rpmbuild/SRPMS/spice-xpi-2.5-1.fc15.src.rpm rpmbuild/RPMS/x86_64/spice-xpi-*
spice-xpi.src: W: invalid-url Source0: spice-xpi-2.5.tar.bz2
spice-xpi.x86_64: W: incoherent-version-in-changelog 2.5.1 ['2.5-1.fc15', '2.5-1']
3 packages and 0 specfiles checked; 0 errors, 2 warnings.
 Both will need to be fixed:
 -now that we've an official tarbal up please replace Source0 with:
  Source0: http://spice-space.org/download/releases/%{name}-%{version}.tar.bz2
 -The version in the changelog should be 2.5-1 not 2.5.1
 -Note that you're supposed to bump the release field each time you make
  changes, even during review so the next changelog entry should have:
  2.5-2 (and Release: near the top should be 2)

- please drop the tarname and tarversion macro-s they are identical to Name
  resp Release and rpm automatically defines %{name} and %{version} for these.

- License: should be: "MPLv1.1 or GPLv2+ or LGPLv2+"

- please change the URL to http://spice-space.org

- please drop BuildRoot, it is not needed in recent Fedora (no in epel-6 or
  newer)

- likewise drop the "rm -rf $RPM_BUILD_ROOT" from %install and the entire %clean
  section

- please drop the gcc-c++ BuildRequires, gcc-c++ is part of the default buildroot, see: http://fedoraproject.org/wiki/PackagingGuidelines#BuildRequires (and then exceptions list)

- please add a comment explaining why ExclusiveArch is used in this case simply
  add the following line above the ExclusiveArch line:
# https://bugzilla.redhat.com/show_bug.cgi?id=613529

- Please drop the following Requires:
  -log4cpp, this is a used library, rpm automatically generates dependencies
   for dynamically linked libraries
  -firefox, this plugin should be usable in other browsers too

- please drop the " -n %{tarname}-%{tarversion}" arguments to %setup,
  %setup uses " -n %{name}-%{version}" as default

- make dist has already generated configure and Makefile.in, etc. No need to
  redo that, please drop the following calls from %build:
aclocal
libtoolize --automake
autoheader
automake --add-missing
autoconf

- with these dropped the following BuildRequires can also be removed:
BuildRequires:  automake
BuildRequires:  libtool
BuildRequires:  autoconf

- the standard for %defattr used in Fedora is:
%defattr(-,root,root,-)

- it is convention to have the %doc line as the first line after %defattr in
  %files

Regards,

Hans
Comment 4 Peter Hatina 2011-06-08 08:18:22 EDT
Now, it should be fixed.

SRPM URL: http://www.stud.fit.vutbr.cz/~xhatin01/spice-xpi/spice-xpi-2.5-2.fc15.src.rpm
Comment 5 Hans de Goede 2011-06-08 09:03:09 EDT
Looks good now, approved. If you let me know your FAS account name I'll add you to the packagers group and sponsor you. For more info on getting a FAS account if you don't have one already and the next steps after sponsoring, see:
http://fedoraproject.org/wiki/PackageMaintainers/Join
Comment 6 Peter Hatina 2011-06-09 02:54:54 EDT
My fedora account name is "phatina".
Comment 7 Hans de Goede 2011-06-09 03:29:04 EDT
OK, I've added you to the packagers group and sponsored you, next step is to create a package scm admin request, so that a new module can be created in pkg git and bugzilla for spice-xpi. See:
http://fedoraproject.org/wiki/Package_SCM_admin_requests

Note it may take up to an hour for your new rights to propagate, so if you cannot set the fedora-cvs flag, that is why.
Comment 8 Peter Hatina 2011-06-09 05:30:40 EDT
New Package SCM Request
=======================
Package Name: spice-xpi
Short Description: Spice extension for Mozilla allows the client to be used from a web browser.
Owners: phatina
Branches: f14 f15
InitialCC: jwrdegoede
Comment 9 Gwyn Ciesla 2011-06-09 05:36:19 EDT
Git done (by process-git-requests).
Comment 10 Fedora Update System 2011-06-09 07:59:34 EDT
spice-xpi-2.5-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/spice-xpi-2.5-2.fc15
Comment 11 Fedora Update System 2011-06-09 08:15:28 EDT
spice-xpi-2.5-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/spice-xpi-2.5-2.fc14
Comment 12 Fedora Update System 2011-06-11 00:28:18 EDT
spice-xpi-2.5-2.fc15 has been pushed to the Fedora 15 testing repository.
Comment 13 Fedora Update System 2011-06-21 13:11:23 EDT
spice-xpi-2.5-2.fc14 has been pushed to the Fedora 14 stable repository.
Comment 14 Fedora Update System 2011-06-21 13:35:40 EDT
spice-xpi-2.5-2.fc15 has been pushed to the Fedora 15 stable repository.

Note You need to log in before you can comment on or make changes to this bug.