Bug 620862 - Review Request: python-newt_syrup - Newt Syrup is an app framework built on top of Newt
Summary: Review Request: python-newt_syrup - Newt Syrup is an app framework built on t...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Steve Traylen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-08-03 16:29 UTC by Darryl L. Pierce
Modified: 2015-06-22 00:07 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-08-06 19:21:07 UTC
Type: ---
Embargoed:
steve.traylen: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Darryl L. Pierce 2010-08-03 16:29:10 UTC
Spec URL: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup.spec
SRPM URL: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-1.fc13.src.rpm
Description: Newt Syrup is an app framework built on top of Newt

Comment 1 Steve Traylen 2010-08-03 18:18:37 UTC
Immediate comments:

1)
%define unmangled_version 0.1.0
%define unmangled_version 0.1.0

is there twice?

2)
Requires:       python(abi) >= 2.6

is not needed , it will be added anyway.

3)

/usr/lib/python2.6/site-packages/src
/usr/lib/python2.6/site-packages/src/__init__.py
/usr/lib/python2.6/site-packages/src/__init__.pyc
/usr/lib/python2.6/site-packages/src/__init__.pyo
/usr/lib/python2.6/site-packages/src/configscreen.py
/usr/lib/python2.6/site-packages/src/configscreen.pyc
/usr/lib/python2.6/site-packages/src/configscreen.pyo
/usr/lib/python2.6/site-packages/src/menuscreen.py
/usr/lib/python2.6/site-packages/src/menuscreen.pyc
/usr/lib/python2.6/site-packages/src/menuscreen.pyo

The "src" directory looks a bit odd, is that intended?

Steve.

Comment 2 Darryl L. Pierce 2010-08-03 19:59:22 UTC
(In reply to comment #1)
> Immediate comments:
> 
> 1)
> %define unmangled_version 0.1.0
> %define unmangled_version 0.1.0
> 
> is there twice?

Hadn't realized it was pasted twice. I've removed it.

> 2)
> Requires:       python(abi) >= 2.6
> 
> is not needed , it will be added anyway.

Removed it.

> 3)
> 
> /usr/lib/python2.6/site-packages/src
> /usr/lib/python2.6/site-packages/src/__init__.py
> /usr/lib/python2.6/site-packages/src/__init__.pyc
> /usr/lib/python2.6/site-packages/src/__init__.pyo
> /usr/lib/python2.6/site-packages/src/configscreen.py
> /usr/lib/python2.6/site-packages/src/configscreen.pyc
> /usr/lib/python2.6/site-packages/src/configscreen.pyo
> /usr/lib/python2.6/site-packages/src/menuscreen.py
> /usr/lib/python2.6/site-packages/src/menuscreen.pyc
> /usr/lib/python2.6/site-packages/src/menuscreen.pyo
> 
> The "src" directory looks a bit odd, is that intended?

I've now moved these up a level.

New SPEC: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup.spec
New SRPM: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-2.fc13.src.rpm

Comment 3 Terje Røsten 2010-08-03 20:23:13 UTC
Darryl, are you sponsored? There are some strangeness in your spec file.
Are you upstream maintainer too?

Comment 4 Terje Røsten 2010-08-03 20:32:37 UTC
OK, I see you have some packages under your belt. 

Quick comments:

%define name newt_syrup
%define version 0.1.0
%define unmangled_version 0.1.0
%define release 2

All this is just noise, remove all.

%define docdir %{python_sitelib}/%{name}-%{version}/doc

This is evil, undone


Summary:        Newt Syrup is an app framework built on top of Newt

Name:           python-%{name}
Version:        %{version}
Release:        %{release}%{?dist}
License:        LGPLv2

See over

Vendor: Darryl L. Pierce <dpierce>

Remove, Fedora is the vendor or all packages.
Who has told you to add this?

Url:            http://newt-syrup.fedorahosted.org/

Source: http://mcpierce.fedorapeople.org/rpms/newt_syrup-%{unmangled_version}.tar.gz

You are upstream for this package? How is the connection betwen

http://newt-syrup.fedorahosted.org/ and this tarball?

Group:          Development/Libraries
BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-buildroot


Prefix: %{_prefix}

What? Why are you defining prefix?

BuildArch:      noarch
Requires:       newt >= 0.52.11
BuildRequires:  python-devel python-setuptools

%description
Newt Syrup is an app framework built on top of Newt.

Some more words here would be nice.


%prep
%setup -q -n newt_syrup-%{version}

%build
%{__python} setup.py build

%install
rm -rf $RPM_BUILD_ROOT
%{__python} setup.py install -O1 --skip-build --root $RPM_BUILD_ROOT

%clean
rm -rf $RPM_BUILD_ROOT

%files
%defattr(-,root,root)

change to 

%defattr(-,root,root,-)


%doc AUTHORS
%doc ChangeLog
%doc COPYING
%{python_sitelib}/*

You use three lines to list %doc, but a single wild card to list the 
important stuff? Please be more explicit.

Do it simple for a quick review and provide a koji scratch build.

Comment 5 Darryl L. Pierce 2010-08-04 11:55:14 UTC
(In reply to comment #3)
> Darryl, are you sponsored? There are some strangeness in your spec file.
> Are you upstream maintainer too?    

Yes to both questions.

Comment 6 Darryl L. Pierce 2010-08-04 12:13:06 UTC
(In reply to comment #4)
<snip applied changes>

> Url:            http://newt-syrup.fedorahosted.org/
> 
> Source:
> http://mcpierce.fedorapeople.org/rpms/newt_syrup-%{unmangled_version}.tar.gz
> 
> You are upstream for this package? How is the connection betwen
> 
> http://newt-syrup.fedorahosted.org/ and this tarball?

The fedorahosted.org website is the project website. mcpierce.fedorapeople.org is where the tarballs will be placed.

> 
> Group:          Development/Libraries
> BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-buildroot
> 
> 
> Prefix: %{_prefix}
> 
> What? Why are you defining prefix?

The initial spec file was one generated by the python-setuptools with a few changes from me. I've applied more changes per your request and made not to be more observant of them in future.

<snip more applied changes>

Updated SPEC:  http://mcpierce.fedorapeople.org/rpms/python-newt_syrup.spec
Updated SRPM:  http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-3.fc13.src.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2379105

Comment 7 Steve Traylen 2010-08-04 13:16:35 UTC
Hi Darryl,

Looks like you lost the module completely:

$ rpm -pql ../RPMS/noarch/python-newt_syrup-0.1.0-3.fc13.noarch.rpm 
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info/PKG-INFO
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info/SOURCES.txt
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info/dependency_links.txt
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info/requires.txt
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info/top_level.txt
/usr/share/doc/python-newt_syrup-0.1.0
/usr/share/doc/python-newt_syrup-0.1.0/AUTHORS
/usr/share/doc/python-newt_syrup-0.1.0/COPYING
/usr/share/doc/python-newt_syrup-0.1.0/ChangeLog

 Steve

Comment 8 Terje Røsten 2010-08-04 13:53:45 UTC
> The initial spec file was one generated by the python-setuptools with a few
> changes from me. 

Ok, I would recommend using a python package already in Fedora as template/example that would will save you some work.


You moved the src/ dir to . in the -2 release, I wonder if you really want to change src/ to  newt_syrup/, then things will work as expected.

Comment 9 Darryl L. Pierce 2010-08-04 14:51:32 UTC
(In reply to comment #8)
> > The initial spec file was one generated by the python-setuptools with a few
> > changes from me. 
> 
> Ok, I would recommend using a python package already in Fedora as
> template/example that would will save you some work.
> 
> You moved the src/ dir to . in the -2 release, I wonder if you really want to
> change src/ to  newt_syrup/, then things will work as expected.    

Thanks for the suggestion, it'll hopefully save me some frustration with this. I took python-mwlib and used it as a guide and I think everything's good now.

SPEC: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup.spec
SRPM: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-4.fc13.src.rpm
SBLD: http://koji.fedoraproject.org/koji/taskinfo?taskID=2379575

Comment 10 Steve Traylen 2010-08-05 14:00:06 UTC
Hi,
Yes the .spec is looking much more familiar and obvious now.


- Package meets naming and packaging guidelines
Yes.
- Spec file matches base package name.
Yes. python-newt_syrop.

- Spec has consistant macro usage.
Yes.
- Meets Packaging Guidelines.
Yes.
- License
Yes. LGPLv2
- License field in spec matches
NO. The license is LGPLv2+
- License file included in package
COPYING FILE is there
- Spec in American English
Yes
- Spec is legible.
Yes
- Sources match upstream md5sum:
No They don't


- Package needs ExcludeArch
It does not.
- BuildRequires correct
It is.
- Spec handles locales/find_lang
No langs present.
- Package is relocatable and has a reason to be.
Not relocatable.
- Package has %defattr and permissions on files is good.
It does.
- Package has a correct %clean section.
It does.
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
It does though not really needed these days.
- Package is code or permissible content.
Yes
- Doc subpackage needed/used.
No large docs present.
- Packages %doc files don't affect runtime.
It will run.
- Headers/static libs in -devel subpackage.
No header files.
- Spec has needed ldconfig in post and postun
No librarires.
- .pc files in -devel subpackage/requires pkgconfig
No pkgconfig files.
- .so files in -devel subpackage.
No .so files.
- -devel package Requires: %{name} = %{version}-%{release}
Not applicable.
- .la files are removed.
Not applicable.
- Package is a GUI app and has a .desktop file
No gui
- Package compiles and builds on at least one arch.
Yes on mine.
- Package has no duplicate files in %files.
None.
- Package doesn't own any directories other packages own.
It does not
- Package owns all the directories it creates.
Yes :
/usr/lib/python2.6/site-packages/newt_syrup
/usr/lib/python2.6/site-packages/newt_syrup-0.1.0-py2.6.egg-info

- No rpmlint output.

$ rpmlint python-newt_syrup.spec ../SRPMS/python-newt_syrup-0.1.0-4.fc13.src.rpm ../RPMS/noarch/python-newt_syrup-0.1.0-4.fc13.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

rpmlint clean.

- final provides and requires are sane:

requires: 
  newt >= 0.52.11
  python(abi) = 2.6

provides: 
  python-newt_syrup = 0.1.0-4.fc13

SHOULD Items:

Issues:

1. The license is I believe LGPLv2+ rather than LGPLv2

2. Sources don't match:

$ rpm -Uvh http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-4.fc13.src.rpm
Retrieving http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-4.fc13.src.rpm
[steve@bottom SPECS]$ md5sum ~/rpmbuild/SOURCES/newt_syrup-0.1.0.tar.gz 
8b4292dcc6f259043c27eebfabaf233b  /home/steve/rpmbuild/SOURCES/newt_syrup-0.1.0.tar.gz

But:

$ wget http://mcpierce.fedorapeople.org/rpms/newt_syrup-0.1.0.tar.gz
--2010-08-05 15:54:12--  http://mcpierce.fedorapeople.org/rpms/newt_syrup-0.1.0.tar.gz
Resolving mcpierce.fedorapeople.org... 128.197.185.45
Connecting to mcpierce.fedorapeople.org|128.197.185.45|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 22042 (22K) [application/x-gzip]
Saving to: “newt_syrup-0.1.0.tar.gz”

100%[=============================================>] 22,042      78.4K/s   in 0.3s    

2010-08-05 15:54:12 (78.4 KB/s) - “newt_syrup-0.1.0.tar.gz” saved [22042/22042]

[steve@bottom SPECS]$ md5sum newt_syrup-0.1.0.tar.gz 
712e128eb955d56f242e57cdd6f414e1  newt_syrup-0.1.0.tar.gz
[steve@bottom SPECS]$ 


8b4292dcc6f259043c27eebfabaf233b != 712e128eb955d56f242e57cdd6f414e1


So other than the source matching looking good.

Steve.

Comment 11 Darryl L. Pierce 2010-08-05 14:44:28 UTC
(In reply to comment #10)
> - License field in spec matches
> NO. The license is LGPLv2+

Fixed.

> - Sources match upstream md5sum:
> No They don't

Uploaded the correct tarball to my fp.o website.

SPEC: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup.spec
SRPM: http://mcpierce.fedorapeople.org/rpms/python-newt_syrup-0.1.0-5.fc13.src.rpm
SBLD: http://koji.fedoraproject.org/koji/taskinfo?taskID=2381854

Comment 12 Steve Traylen 2010-08-06 13:57:39 UTC
Looks good to go

$ md5sum newt_syrup-0.1.0.tar.gz ../SOURCES/newt_syrup-0.1.0.tar.gz 
7a9ab48df7f87d2c9121cd201d23b607  newt_syrup-0.1.0.tar.gz
7a9ab48df7f87d2c9121cd201d23b607  ../SOURCES/newt_syrup-0.1.0.tar.gz

APPROVED.

Comment 13 Darryl L. Pierce 2010-08-06 14:12:35 UTC
New Package SCM Request
=======================
Package Name: python-newt_syrup
Short Description: Newt Syrup is an app framework built on top of Newt

Owners: mcpierce
Branches: F13 F14 EL5 EL6
InitialCC:

Comment 14 Jason Tibbitts 2010-08-06 17:28:08 UTC
Git done (by process-git-requests).

Note that the SCM request is garbled; it has a blank line in the middle.  I
fixed it up.

Comment 15 Darryl L. Pierce 2010-08-06 19:21:07 UTC
(In reply to comment #14)
> Git done (by process-git-requests).
> 
> Note that the SCM request is garbled; it has a blank line in the middle.  I
> fixed it up.    

Thanks, Jason. :)


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