Bug 620862
| Summary: | Review Request: python-newt_syrup - Newt Syrup is an app framework built on top of Newt | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Darryl L. Pierce <dpierce> |
| Component: | Package Review | Assignee: | Steve Traylen <steve.traylen> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | fedora-package-review, notting, steve.traylen, terje.rosten, tross |
| Target Milestone: | --- | Flags: | steve.traylen:
fedora-review+
j: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2010-08-06 19:21:07 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: | |||
|
Description
Darryl L. Pierce
2010-08-03 16:29:10 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. (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 Darryl, are you sponsored? There are some strangeness in your spec file. Are you upstream maintainer too? 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.
(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. (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 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 > 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.
(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 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.
(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 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. 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: 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. (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. :) |