Bug 182305
Summary: | Review Request: pyspi - Python bindings for AT-SPI | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Zack Cerza <zcerza> |
Component: | Package Review | Assignee: | Toshio Kuratomi <toshio> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | dmalcolm, ezannoni, jrb, rousseau, toshio |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-04-17 17:24:18 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 182306 |
Description
Zack Cerza
2006-02-21 18:35:32 UTC
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> - 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. I'll sponsor. Apply for cvsextras in the Fedora accounts system. 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. 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. 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. 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. 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. :) 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. 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 Thanks! Here's the updated SRPM: http://people.redhat.com/zcerza/dogtail/releases/FE/pyspi-0.5.4-2.src.rpm 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. Thanks again, and sorry for the delay. I've just queued a build for the devel branch, and requested an FC5 branch. Builds succeeded! yay. |