Bug 235526 - Review Request: pyxattr - Extended attributes library wrapper for Python
Review Request: pyxattr - Extended attributes library wrapper for Python
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-06 14:29 EDT by Marcin Zajaczkowski
Modified: 2007-11-30 17:12 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-04-28 15:57:39 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marcin Zajaczkowski 2007-04-06 14:29:12 EDT
Spec URL: http://timeoff.wsisiz.edu.pl/rpms/python-xattr/python-xattr.spec
SRPM URL: http://timeoff.wsisiz.edu.pl/rpms/python-xattr/python-xattr-0.2.1-1.src.rpm
Description: Python extension module wrapper for libattr. It allows to query, list, add and remove extended attributes from files and directories.

It's a recommended runtime dependency for rdiff-backup (already in Fedora Extras).
Comment 1 Marcin Zajaczkowski 2007-04-06 14:36:59 EDT
Added FE-NEW dependency.
Comment 2 Kevin Fenzi 2007-04-17 23:14:09 EDT
See below - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
c2c28332bbeefc2cf0627ecb4c10c9b0  pyxattr-0.2.1.tar.gz
c2c28332bbeefc2cf0627ecb4c10c9b0  pyxattr-0.2.1.tar.gz.1
OK - BuildRequires correct
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
OK - No rpmlint output.
OK - final provides and requires are sane

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version

Issues:

1. Is the name right here? According to the guidelines if the upstream has 'py'
in it
you can just use that name:
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-8756a3bce652c376d7ba3908461b638784b6952d
so perhaps pyxattr would be acceptable instead of python-xattr?

2. Upstream doesn't seem to active, but you might ask if they can include
a copy of the GPL with the package. Not a blocker.

3. The Source URL should probibly be:
http://downloads.sourceforge.net/pyxattr/pyxattr-%{version}.tar.gz

4. Do you need the
%defattr(0644,root,root,0755)
or does a normal
%defattr(-,root,root,-)
Install files with the correct permissions?

5. What is the
#Packager removed due to FE requirements
comment about? (This also applies to the pylibacl too)
Comment 3 Marcin Zajaczkowski 2007-04-18 15:57:17 EDT
(In reply to comment #2)
> 1. Is the name right here? According to the guidelines if the upstream has
> 'py' in it you can just use that name:
(...)
> so perhaps pyxattr would be acceptable instead of python-xattr?

You are right. I missed a line about an exception.

> 2. Upstream doesn't seem to active, but you might ask if they can include
> a copy of the GPL with the package. Not a blocker.

Ok, I will.

> 3. The Source URL should probibly be:
> http://downloads.sourceforge.net/pyxattr/pyxattr-%{version}.tar.gz

Changed.

> 5. What is the
> #Packager removed due to FE requirements
> comment about? (This also applies to the pylibacl too)

This spec file was originally made by Dag Wieers. I make a note that removed
packager tag due to FE requirements not because I tried to descrease role of Dag
in a package creation process.


New links:
http://timeoff.wsisiz.edu.pl/rpms/pyxattr/pyxattr.spec
http://timeoff.wsisiz.edu.pl/rpms/pyxattr/pyxattr-0.2.1-2.src.rpm


Thanks for your reviews
Comment 4 Marcin Zajaczkowski 2007-04-18 16:11:19 EDT
Changed package name in a bug summary.
Comment 5 Kevin Fenzi 2007-04-18 23:14:11 EDT
1. ok. 
2. ok. 
3. ok. 
4. ok. 
5. Ah, I see. Well, as long as you credit Dag in the changelog (ie, don't remove
any of his changelog entries) I think thats giving sufficient credit. 

I see no further blockers here, so this package is APPROVED. 
Don't forget to close this review request once the package has been imported and
built. 
Comment 6 Marcin Zajaczkowski 2007-04-20 14:45:45 EDT
New Package CVS Request
=======================
Package Name: pyxattr
Short Description: Extended attributes library wrapper for Python
Owners: mszpak@wp.pl
Branches: FC-5 FC-6
InitialCC: 
Comment 7 Marcin Zajaczkowski 2007-04-28 15:57:39 EDT
It took me some time, but there is a big delay between build and publish
packages in a file repository.

After change of a name I added:
Provides:       python-xattr = %{version}-%{release}
Obsoletes:      python-xattr <= %{version}-%{release}

(python-xattr name was used in DAG repository and package wasn't automatically
updated).

I talked with upstream and he promised to add a file with a license text to a
tarball in the next version (if any :) ).

Thanks for your help.
Comment 8 Marcin Zajaczkowski 2007-07-07 04:46:53 EDT
Package Change Request
======================
Package Name: pyxattr
New Branches: EL-4 EL-5
Updated EPEL Owners: mszpak@wp.pl,kevin@tummy.com
Comment 9 Kevin Fenzi 2007-07-08 00:55:28 EDT
cvs done.

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