Bug 191218

Summary: Review Request: PyScript - Postscript graphics with Python
Product: [Fedora] Fedora Reporter: Paul Cochrane <paul>
Component: Package ReviewAssignee: Thorsten Leemhuis (ignored mailbox) <bugzilla-sink>
Status: CLOSED DUPLICATE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jspaleta, kevin, michel.salim, sander
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-12-11 05:52:10 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: 201449    

Description Paul Cochrane 2006-05-09 20:08:38 UTC
Spec URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript-0.6.fc4.spec
SRPM URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript-0.6-1.src.rpm
Description: PyScript is a Python module for producing high quality postscript graphics.  Rather than use a GUI to draw a picture, the picture is programmed using Python and the PyScript objects.  This is my first submission to Fedora Extras, and I'm seeking a sponsor.

Comment 1 Sander Hoentjen 2006-05-11 19:46:52 UTC
My first review, and i am not a sponsor

I will just list some of the things you probably want to fix:
- BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}
- n)
- Specfile should be in the format %{name}.spec
- Package is marked as relocatable, is that really needed? It probably isn't so
that has to be fixed (get rid of Prefix). If it really is necessary you need to
explain why.
- some paths are not replaced with RPM macros
- The BuildRoot must be cleaned at the beginning of %install (add rm -rf
$RPM_BUILD_ROOT)



Comment 2 Paul Cochrane 2006-05-12 09:27:37 UTC
(In reply to comment #1)
Thanks for the review!

> - BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u}
> - n)
Corrected.

> - Specfile should be in the format %{name}.spec
Corrected.

> - Package is marked as relocatable, is that really needed? It probably isn't so
> that has to be fixed (get rid of Prefix). If it really is necessary you need to
> explain why.
Removed Prefix: attribute from spec file and replaced %{prefix} with the actual
path.  This, however, makes rpmlint give a lot of errors saying that the path is
hardcoded.  Is this correct?

> - some paths are not replaced with RPM macros
Sorry, I'm not 100% sure which ones are incorrect.

> - The BuildRoot must be cleaned at the beginning of %install (add rm -rf
> $RPM_BUILD_ROOT)
Corrected.

The new SPEC and SRPM file URLs are:
Spec URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript.spec
SRPM URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript-0.6-1.src.rpm

Comment 3 Sander Hoentjen 2006-05-12 10:04:13 UTC
no time to really look at the specfile now but read
http://fedoraproject.org/wiki/Extras/RPMMacros
there you will find the correct macro's to replace the paths with. ( %{_libdir} )
Also increase your release number everytime you make a change, even during
review and put your changes in the changelog.
also see
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b7d622f4bb245300199c6a33128acce5fb453213
for how to format the changelog entries

Good luck

Comment 4 Paul Cochrane 2006-05-12 11:26:54 UTC
(In reply to comment #3)
Thanks again for the help!  :-)

I have updated the SRPM and SPEC files as advised, they are at:
Spec URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript.spec
SRPM URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript-0.6-2.src.rpm

Comment 5 Sander Hoentjen 2006-05-12 16:46:08 UTC
Ok, I had a little bit more time now, sorry for not giving all directions at
once but i am also still learning.
(In reply to comment #4)
A couple of things:
1) %prep section:
make it:
%setup -q
I don't know if this is a must, but I think it becomes easier if you do this.
First of all you have one line in %prep instead of 3, second you can remove a
lot of %{name}-%{version} so the specfile becomes easier to read

2) I read http://fedoraproject.org/wiki/Packaging/Python , you should do that
too. I hinted to replace paths with libdir but in this page you can read about
%{python_sitelib}, use this instead.

3) use wildcards in the %files section, instead of listing all files seperatly
(see the url in 2 for an example)

4) python setup.py install --prefix=$RPM_BUILD_ROOT/usr still has /usr hardcoded

5) time is up again for me, just a general hint that you can find a lot of info
in the fedora extras wiki.

6) don't take everything i say for granted, i could be wrong sometimes so if you
think i might be just say so.

Comment 6 Paul Cochrane 2006-05-15 10:09:44 UTC
(In reply to comment #5)
> A couple of things:
> 1) %prep section:
> make it:
> %setup -q
Done.  Spec file is a lot cleaner now.

> 2) I read http://fedoraproject.org/wiki/Packaging/Python , you should do that
> too. I hinted to replace paths with libdir but in this page you can read about
> %{python_sitelib}, use this instead.
Done.

> 3) use wildcards in the %files section, instead of listing all files seperatly
> (see the url in 2 for an example)
Done as much as possible.  There are some files in the lib/ directory of the
PyScript source tree that we don't yet want distributed so I've kept these files
specified explicitly.

> 4) python setup.py install --prefix=$RPM_BUILD_ROOT/usr still has /usr hardcoded
Fixed.
 
Thank you again for your time!

I have updated the SRPM and SPEC files, they are at:
Spec URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript.spec
SRPM URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript-0.6-3.src.rpm

Comment 7 Sander Hoentjen 2006-05-15 11:48:59 UTC
Hi, this package seems ok to me now, except for 2 small things:
- requires doesn't have to have python, that will be taken care of automatically
- changelog entries should be in the format:
* Wed May 14 2003 Joe Packager <joe at gmail.com> - 0:1.0-0.fdr.2
you are missing the version in the end.

Other then that you now have to wait* until some sponsor reviews this package.

*) If you just wait, chances of getting sponsored aren't that high, so try to
review some other packages, or submit some more yourself. (In short, show that
you are worthy of sponsorship, like i am trying to do myself)

Comment 8 Paul Cochrane 2006-05-15 12:24:14 UTC
(In reply to comment #7)
> - requires doesn't have to have python, that will be taken care of automatically

Removed.

> - changelog entries should be in the format:
> * Wed May 14 2003 Joe Packager <joe at gmail.com> - 0:1.0-0.fdr.2
> you are missing the version in the end.

Got the version number working such that rpmlint is now happy.

Many thanks for your help, it is greatly appreciated.  :-)

As per usual, the updated files are at:
Spec URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript.spec
SRPM URL: http://dl.sourceforge.net/sourceforge/pyscript/pyscript-0.6-4.src.rpm

Comment 9 Kevin Fenzi 2006-08-27 21:40:46 UTC
Hey Paul. I see only this one submission from you in bugzilla, so it's kinda 
hard to know if you are ready to be sponsored. 

Can you take a look at: 
http://fedoraproject.org/wiki/Extras/HowToGetSponsored
Adding some review comments to other bugs to show you know the guidelines, as 
well as submitting a few more packages would help a lot. 


Comment 10 Paul Cochrane 2006-11-22 20:50:25 UTC
Kevin,

Thanks for your comments, and many apologies for not having replied sooner.  To 
be totally honest I didn't know what to say, and have only just got around to 
cleaning up my inbox.

Basically, what I would like to do is to get PyScript into FedoraExtras but not 
necessarily be the actual maintainer of the package, rather stay the upstream 
author.  Is there another way to do this other than getting sponsored?  It 
would be fantastic to get the package into Fedora so that more people have 
access to the package.  Is it possible to find someone else who already 
packages similar software for FedoraExtras who would be able to perfom this 
role?

Thanks heaps in advance,

Paul

Comment 11 Kevin Fenzi 2006-11-28 04:01:26 UTC
Hey Paul. 

You might try posting to the fedora-extras list 
(see: http://www.redhat.com/mailman/listinfo/fedora-extras-list )

and seeing if you can find anyone who's interested in maintaining this package...



Comment 12 Jef Spaleta 2006-12-03 02:56:36 UTC
I might be interested in being the primary maintainer of this package.  I
believe that I can list you as co-maintainer so that you get bugzilla tickets.
As things move forward, until there are more fine-grained access controls i can
be the gatekeeper for any fedora extras cvs commits you want to make to the
package.  You can build a public track-record of 'right action' by communicating
with me through bugzilla and make it easier to gain contributor status.

What is the most recent spec and srpm that I should look at.

And most importantly, has there been a previous review that would have counted
as a full review if you hadn't needed sponsorship.  I want to see if I can
contact a reviewer who is already looked at this so when I re submit it, we can
streamline the timetable and get this in quickly.

-jef

Comment 13 José Matos 2006-12-03 18:25:39 UTC
(In reply to comment #12)
> And most importantly, has there been a previous review that would have 
counted
> as a full review if you hadn't needed sponsorship.  I want to see if I can
> contact a reviewer who is already looked at this so when I re submit it, we 
can
> streamline the timetable and get this in quickly.

  If wants to take the package with the original submiter as co-maintainer I 
will proceed with the (official) review.

> -jef



Comment 14 Paul Cochrane 2006-12-04 07:54:23 UTC
(In reply to comment #12)
> I might be interested in being the primary maintainer of this package.  I
> believe that I can list you as co-maintainer so that you get bugzilla tickets.
> As things move forward, until there are more fine-grained access controls i 
can
> be the gatekeeper for any fedora extras cvs commits you want to make to the
> package.  You can build a public track-record of 'right action' by 
communicating
> with me through bugzilla and make it easier to gain contributor status.
That would be great!

> What is the most recent spec and srpm that I should look at.
See comment #7 for links to the current spec and srpm.

> And most importantly, has there been a previous review that would have counted
> as a full review if you hadn't needed sponsorship.  I want to see if I can
> contact a reviewer who is already looked at this so when I re submit it, we 
can
> streamline the timetable and get this in quickly.
The review is as documented in the early comments of this bug ticket.  The 
person who helped me was Sander Hoentjen, and we ironed out all the problems 
with the spec.  Everything seemed to be fine after that much review.

Thank you very much for your offer, and being co-maintainer would be a very 
good way to get any required changes into the package so that it can be 
included in Fedora Extras.

Paul

Comment 15 Jef Spaleta 2006-12-07 09:22:21 UTC
Okay I have the srpm from comment 8 now. I'll have time to look over it Friday.
 If I don't have any problems with it, I'll open a new request ticket and close
this bug out with a note as a duplicate to my ticket.

We should be able to get this done over the weekend.

-jef

Comment 16 Jef Spaleta 2006-12-11 05:52:10 UTC
Okay I have opened a new review ticket. I am closing this ticket as a duplicate
of my new ticket. I did make changes to the spec file to bring the spec into
line with current python packaging policy. ghosting of the pyo's is no longer
prefered according to the python specific packaging guidance.

If you have the ability to do a full review, please head on over to 
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219119

-jef

*** This bug has been marked as a duplicate of 219119 ***

Comment 17 Kevin Fenzi 2006-12-22 03:52:18 UTC
Moving blockers to FE-DEADREVIEW.