[Bug 542715] Review Request: RabbitVCS - Easy version control

Sat Mar 20 16:56:59 UTC 2010

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=542715

Kalev Lember <kalev at smartlink.ee> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|nobody at fedoraproject.org    |kalev at smartlink.ee
               Flag|                            |fedora-review?

--- Comment #12 from Kalev Lember <kalev at smartlink.ee> 2010-03-20 12:56:52 EDT ---
This is mostly just nitpicking, but python_sitearch macro isn't used anywhere
in this spec, so there's no need to define it in here. Also, the newly-approved
Fedora python guidelines suggest to wrap the python_sitearch/python_sitelib
definitions inside a conditional [1] like this:
%if ! (0%{?fedora} > 12 || 0%{?rhel} > 5)
... insert python_sitelib definition here
%endif

[1] https://fedoraproject.org/wiki/Packaging:Python#Macros

> Summary:        Integrated Subversion support for Nautilus
This is the summary for the main package which just contains the core
components, but not the actual Nautilus integration. For Nautilus support one
has to install rabbitvcs-nautilus instead. Can you rewrite the summary so that
it doesn't sound like it's Nautilus extension package? Something like
"Graphical user interface to version control systems" (I stole it from Debian
package [2])

[2]
http://svn.debian.org/viewsvn/python-modules/packages/rabbitvcs-core/trunk/debian/control?view=markup

> URL:            http://code.google.com/p/rabbitvcs/
Maybe http://www.rabbitvcs.org would be better?

> BuildRequires:  pygtk2-devel >= 2.12
Is pygtk2-devel really needed at build time?

> BuildRequires:  python-devel >= 2.5
You can drop the >= 2.5. All current Fedora versions have at least Python 2.6.
Also, Fedora python guidelines suggest to BR python2-devel, not python-devel.

> Provides:       nautilussvn = 0.12
> Provides:       rabbitvcs-core
> Obsoletes:      nautilussvn <= 0.13

For preserving upgrade path from the original vendor's rpms, you need
obsoletes/provides for nautilussvn and rabbitvcs-core. Something like this:
Provides:       nautilussvn = %{version}-%{release}
Obsoletes:      nautilussvn < 0.13
Provides:       rabbitvcs-core = %{version}-%{release}
Obsoletes:      rabbitvcs-core < 0.13-2

Note that rabbitvcs-core-0.13-1.fc12 is the last version available at
rabbitvcs.org, so we need to make sure we obsolete that.

> Requires:       pygtk2 >= 2.12
No need to specify the version, as pygtk2-2.12 is available since F-8.

I think you are missing Requires: pygtk2-libglade here, can you verify that?

%package nautilus
> Summary:        rabbitvcs plugin for nautilus
Summary should be capitalized. I have some links to various Debian rabbitvcs
packages with really nicely written summaries/descriptions. Take a look, maybe
there's something you can reuse here:
http://svn.debian.org/viewsvn/python-modules/packages/rabbitvcs-core/trunk/debian/control?view=markup
http://svn.debian.org/viewsvn/python-modules/packages/rabbitvcs-nautilus/trunk/debian/control?view=markup
http://svn.debian.org/viewsvn/python-modules/packages/rabbitvcs-gedit/trunk/debian/control?view=markup
http://svn.debian.org/viewsvn/python-modules/packages/rabbitvcs-core/trunk/debian/control?view=markup

> %package nautilus
> Requires:       rabbitvcs-core = %{version}
If you don't have rabbitvcs-core package (currently it's just a virtual
provide), please require the actual package name instead (rabbitvcs). Also, the
require should be tightened with -%{release} for subpackages which are built
from the same srpm:
Requires:       rabbitvcs = %{version}-%{release}

> %package nautilus
> Requires:       nautilus >= 2.22
> Requires:       nautilus-python >= 0.5.2
> Requires:       dbus-python > 0.80
The nautilus-python versioned require is needed, as the older version which
shipped in F-11 and F-12 GA is buggy. But are the versioned requires really
needed for others?

> %package nautilus-old
We don't have nautilus-1 in current Fedora releases and this subpackage should
just be dropped.

> %package thunar
> Requires:       thunarx-python >= 0.2.0

thunarx-python isn't in Fedora. Have you filed a review request for that yet?
If yes, please mark thunarx-python review request as blocking this one. If not,
lets drop or comment out that subpackage as well.

> %setup -n %{name}-%{version}
Please use the -q argument. -n %{name}-%{version} is the default, so you can
just use:
%setup -q

> %{__python} setup.py install --root $RPM_BUILD_ROOT
Please add --skip-build:
%{__python} setup.py install --skip-build --root $RPM_BUILD_ROOT

> mkdir -p $RPM_BUILD_ROOT%{_libdir}/nautilus/extensions-2.0/python/
> mv clients/nautilus/%{title}.py $RPM_BUILD_ROOT%{_libdir}/nautilus/extensions-2.0/python/%{title}.py

You are installing in %{_libdir}, but the whole package is marked as noarch.
Subpackages which install into %{_libdir} certainly can't be noarch.

I'm not sure how it's best to fix this. The main package (old -core) would be a
good candidate for being noarch, but I don't think there's a way to mark that
noarch without making the whole package build noarch.

I can think of two options:
a) not mark the whole package as noarch, but instead introduce the -core
subpackage and mark that one as noarch, or
b) not use noarch at all.

Any ideas? You mentioned that Toshio helped you with this package, maybe he can
help with the noarch problem?

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