Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: python-rocket - Modern, multi-threaded, comet-friendly WSGI web server
https://bugzilla.redhat.com/show_bug.cgi?id=639874
Summary: Review Request: python-rocket - Modern, multi-threaded, comet-friendly WSGI web server Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: ilia.cheishvili@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://github.com/icheishvili/rpms/raw/master//python-rocket.spec SRPM URL: http://github.com/icheishvili/rpms/raw/master//python-rocket-1.1.1-1.fc13.sr... Description: This is a modern, multi-threaded, comet-friendly WSGI web server for python. I am hoping to get it included into Extras.
The Rocket web server is a server designed to handle the increased needs of modern web applications implemented in pure Python. It can serve WSGI applications and static files. Rocket has the ability to be extended to handle different types of networked request-response jobs. Rocket runs on cPython 2.5- 3.x and Jython 2.5 (without the need to run through the 2to3 translation tool). Rocket is similar in purpose to Cherrypy's Wsgiserver but with added flexibility, speed and concurrency.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
Thomas Spura tomspur@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tomspur@fedoraproject.org
--- Comment #1 from Thomas Spura tomspur@fedoraproject.org 2010-10-20 07:45:31 EDT --- (Didn't look much to this package...)
Why don't you build a python3 subpackage, if it's possible? At least your description says so...
see: https://fedoraproject.org/wiki/Packaging:Python
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #2 from Ilia Cheishvili ilia.cheishvili@gmail.com 2010-11-21 23:56:14 EST --- A very good point. I will see about doing this. Thanks for the wiki link.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
Jeffrey Ness jeffrey.ness@rackspace.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jeffrey.ness@rackspace.com
--- Comment #3 from Jeffrey Ness jeffrey.ness@rackspace.com 2011-01-10 15:11:27 EST --- Greetings,
A few things I noticed while looking at your SPEC and Bug Review:
* Every Review should include a rpmlint of both source rpm and rpm binary: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
* Add %{version} to your Source0 tag, this will allow easy version upgrades. http://pypi.python.org/packages/source/r/rocket/Rocket-%%7Bversion%7D.zip
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #4 from Ilia Cheishvili ilia.cheishvili@gmail.com 2011-01-10 23:58:41 EST --- I made all updates suggested:
Spec URL: http://github.com/icheishvili/rpms/raw/master/python-rocket.spec SRPM URL: http://github.com/icheishvili/rpms/raw/master/python-rocket-1.2.2-1.fc14.src...
Also, here's rpmlint output:
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #5 from Jeffrey Ness jeffrey.ness@rackspace.com 2011-01-25 15:55:08 EST --- Here is a bit more detailed review in order to help get the package approved, as mentioned above this is unofficial.
Good: *rpmlint clean * Package follows naming guidelines * Spec file name matches package name * License is MIT in source and spec file * MIT is an open source license * Spec file is legible American English * Source matches upstream: MD5sum b0bfa3bca9d30838c5dec4a083fbd247 * Builds in mock * All build deps satisfied but see below; there's some extra ones. * No locale files that need to be marked with %find_lang * No shared libraries * No bundled libraries * Package is not relocatable * No files listed more than once * All files and directories created by the package owned by the package and no others. * Package contains code, not content. * No large documentation that needs to be in a separate subpackage * Nothing in %doc used at runtime * No GUI application included so no .desktop requirement * All filenames are valid utf-8 * No scriptlets * No file dependencies * No programs so no need for man pages
Needswork: *You should use %global rather than %global
http://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define#c...
Cosmetic: * Speak with upstream about adding LICENSE to source:
http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #6 from Toshio Ernie Kuratomi a.badger@gmail.com 2011-01-25 16:26:35 EST --- Note: In your Requires: section you have an: %if 0%{?with_python3} BuildRequires: python3-setuptools BuildRequires: python3-devel Requires: python3 %else Requires: python %endif
You actually want to change that a little bit. The "Requires: python" should be outside of the conditional since you want it to be a Requirement of the main python-rocket package whether or not a python3 subpackage is built. The Requires: python3 portion should be part of the python3-rocket subpackage further down in the spec file so that only the subpackage has a dependency on python3.
BuildRequires stay at the top because one environment is setup for building all of the subpackages. So there's no need to parcel those out to each subpackage section.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #7 from Ilia Cheishvili ilia.cheishvili@gmail.com 2011-01-31 14:01:56 EST --- Jeffrey and Toshio, thanks for the reviews. I have made the suggested changes. Find the new SPEC and SRPM here:
https://github.com/icheishvili/rpms/raw/master/python-rocket.spec https://github.com/icheishvili/rpms/raw/master/python-rocket-1.2.3-1.fc14.sr...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
Haïkel Guémar karlthered@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |karlthered@gmail.com
--- Comment #8 from Haïkel Guémar karlthered@gmail.com 2012-01-23 16:23:28 EST --- Are you still working on it ? * upstream has released rocket 1.2.4 since * you should request upstream to add a License file * are you planning to maintain this package on EPEL5 ? if not, you should remove any reference to buildroot cleaning and defattr * you should keep updating the changelog, at least when updating upstream releases (some reviewers might indulge that you don't bump release field but nobody will approve this package if you don't maintain properly your changelog)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #9 from Ilia Cheishvili ilia.cheishvili@gmail.com 2012-02-26 23:02:30 EST --- Thanks for pointing this out. Here are the updated versions:
https://github.com/icheishvili/rpms/raw/master/python-rocket.spec https://github.com/icheishvili/rpms/raw/master/python-rocket-1.2.4-1.fc16.sr...
I do have a question for you, however. What do you mean when you say I should take out "any reference to buildroot cleaning and defattr" unless I intend to maintain the package in epel5? Perhaps I'm just not understanding how the two relate to each other.
I have also asked upstream to include a LICENSE file: https://answers.launchpad.net/rocket/+question/188972
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=639874
--- Comment #10 from Haïkel Guémar karlthered@gmail.com 2012-02-27 01:54:33 EST --- 1. %defattr is no more required since RPM 4.4, it is implied since. 2. The Buildroot tag is ignored, the provided buildroot will be automatically cleaned (since F10).
That is valid for all currently supported Fedora branches and EPEL6 branch but not EPEL5. According packaging guidelines, these must be removed unless you plan to maintain your package on EPEL5 (for which you're granted an exception).
package-review@lists.fedoraproject.org