Bug 182305 - Review Request: pyspi - Python bindings for AT-SPI
Review Request: pyspi - Python bindings for AT-SPI
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Toshio Kuratomi
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 182306
  Show dependency treegraph
 
Reported: 2006-02-21 13:35 EST by Zack Cerza
Modified: 2007-11-30 17:11 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-04-17 13:24:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Zack Cerza 2006-02-21 13:35:32 EST
Spec Name or Url: http://people.redhat.com/zcerza/dogtail/releases/FE/pyspi.spec
SRPM Name or Url: http://people.redhat.com/zcerza/dogtail/releases/FE/pyspi-0.5.4-1.src.rpm
Description: Python bindings for AT-SPI. Allows Python programs to act as Assistive Technology Service Providers - i.e. screen readers. pyspi was created for use by dogtail, a GUI test tool and automation framework.

This (and dogtail) are my first packages, and I'm seeking a sponsor.
Comment 1 Toshio Kuratomi 2006-03-04 02:40:19 EST
I cannot sponsor you.  But I thought I'd do a review of the package and maybe
it would help get someone else to look things over and sponsor you sooner.

rpmlint:
src.rpm:
W: pyspi strange-permission pyspi.spec 0600
- You can change the mode to 0644 but this shouldn't harm anything.

x86_64.rpm:
W: pyspi no-version-in-last-changelog
- Changelogs in FE contain package version information like so:
    * Tue Feb 14 2006 Zack Cerza <zcerza@redhat.com> - 0.5.4-1

Needswork:
* Source should be a full URL to the upstream source:
 
http://people.redhat.com/zcerza/dogtail/releases/rpm_inst/%{name}-%{version}.tar.gz

* Description is short.  Perhaps something like this:
'''
  at-spi allows assistive technologies to access GTK-based applications.  It
  exposes the internals of applications for automation, so tools such as screen
  readers and scripting interfaces can query and interact with GUI controls.

  The pyspi binding allows the python language to be used to script at-spi
  aware applications (currently GTK+ based.)
'''

* Requires: at-spi is not needed as rpm finds the dependency automatically.
  Requires: python is not needed in FC4+ (rpm generates a requirement for:
    python(abi) = 2.4
  I'm not sure about FC3 but that's in maintence mode so you probably want
  to drop this explicit requirement from the spec file as well.

Minor:
* The FE preferred BuildRoot is
  %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  The id -u at the end ensures that two people rebuilding the package on a
  multi-user machine will not conflict with each other.
* Usual group for python libraries is Development/Languages
* You may use the %{dist} tag at the end of Release to make releases for
  several Core releases easier.

Good:
Went through the whole checklist.  Here are the highlights:
* Follows naming guidelines
* spec name matches base package name
* License is LGPL and properly reflected in tarball, spec, and package.
* The spec is quite understandable
* Source matches upstream tarball
* Builds on x86_64
* Permissions set correctly
* %clean cleans buildroot
* Consistently uses macros
* Documentation from the tarball is included
* Does not own files owned by another package
* No scriptlets
* Builds in mock
* Not understanding how at-spi is supposed to work, I didn't actually see if
  pyspi does anything useful.  I was able to load it up in the python
  interpreter and instantiate various atspi objects without segfaults,
  introspect them, and generally realize how foreign at-spi is to me.  I'll
  look at dogtail next and see what I can do with the combination of packages.
Comment 2 John Mahowald 2006-03-04 17:43:56 EST
I'll sponsor. Apply for cvsextras in the Fedora accounts system.
Comment 3 Zack Cerza 2006-03-10 14:53:40 EST
Toshio, thanks for the review! I've implemented everything you suggested in CVS,
and I'll make a new release and clear up the URLs soon. I did have some trouble
with the %{dist} tag, as it was showing up in filenames - i.e. not being
replaced by anything. I changed it to %{?dist} as the Canna specfile does, but
now it just gets eliminated in filenames. I'm not sure what that means...

John, thanks for sponsoring me. I just applied for my CVS account.
Comment 4 Zack Cerza 2006-03-10 15:00:17 EST
Toshio, about pyspi: the API it binds is not at all easy to understand, IMO.
Thank CORBA. The only reason it exists is for dogtail. Dogtail's API tries to
make it much more intuitive, among other things.
Comment 5 Toshio Kuratomi 2006-03-13 16:53:20 EST
Heh.  I noticed :-)  dogtail seems like a much easier interface to learn, thanks!

The dist tag should end up empty outside of the Fedora Extras buildsystem.  You
did the right thing by making it optional.

Post the new URLs when you roll a new version out and I'll review again.
Comment 6 Zack Cerza 2006-03-14 16:59:58 EST
Here you go:

http://people.redhat.com/zcerza/dogtail/releases/FE/pyspi-0.5.4-1.src.rpm
http://people.redhat.com/zcerza/dogtail/releases/FE/pyspi.spec

For some reason the specfile inside the SRPM is always 0600 - even though it's
0644 in the tarball.
Comment 7 Zack Cerza 2006-03-17 13:32:14 EST
At http://fedoraproject.org/wiki/Extras/Contributors I'm told to import my
"approved SRPM". Should I consider the above 'approved'? I'll wait to make sure. :)
Comment 8 Toshio Kuratomi 2006-03-17 13:42:36 EST
I need to re-review it.  Shouldn't take long but I've been busy all week.  I
promise to rereview this and dogtail before the weekend is out.
Comment 9 Toshio Kuratomi 2006-03-18 00:03:49 EST
In the future, please update the Release when you create a new version.  It
helps the reviewers to keep track of what issues exist is each version.

7084cf77996c3e62ef928f450f710111  pyspi-0.5.4-1.src.rpm

Mustfix:
* The URL: field was fine last time, it was the Source0: line that needed to
  be changed to have a full URL to the sources.  So change the current spec
  file to this::
    URL: http://people.redhat.com/zcerza/dogtail/
    Source0:
http://people.redhat.com/zcerza/dogtail/releases/pyspi-%{version}.tar.gz

Fixed:
* Changelog now contains release numbers.
* Description is more informative.
* Requires have been trimmed.
* Buildroot, Group, and dist tag were updated.

Still builds in mock.

I'm setting this to APPROVE.  Please make sure you fix the Source0/URL lines and
bump the release before you import the package into cvs.

You're now on Step 8 of::
  http://www.fedoraproject.org/wiki/Extras/NewPackageProcess
Comment 10 Zack Cerza 2006-03-21 19:57:10 EST
Thanks! Here's the updated SRPM:

http://people.redhat.com/zcerza/dogtail/releases/FE/pyspi-0.5.4-2.src.rpm
Comment 11 Toshio Kuratomi 2006-03-26 02:47:25 EST
ad376e847dbd900e784ec17b785da1ce  pyspi-0.5.4-2.src.rpm

Everything's shipshape and ready to go.
Package is APPROVED.

You can now import, edit the owners.list file, and get the package built in the
build system.
Comment 12 Zack Cerza 2006-04-17 13:06:52 EDT
Thanks again, and sorry for the delay. I've just queued a build for the devel
branch, and requested an FC5 branch.
Comment 13 Zack Cerza 2006-04-17 13:24:18 EDT
Builds succeeded! yay.

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