Bug 253355 - Review Request: twill - A simple scripting language for Web browsing
Summary: Review Request: twill - A simple scripting language for Web browsing
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matthias Saou
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2007-08-18 05:45 UTC by Luke Macken
Modified: 2016-09-20 02:37 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-03-06 18:01:39 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Luke Macken 2007-08-18 05:45:23 UTC
Spec URL: http://lmacken.fedorapeople.org/rpms/twill.spec
SRPM URL: http://lmacken.fedorapeople.org/rpms/twill-0.9-0.1.b1.fc7.src.rpm
Description: twill is a simple language that allows users to browse the Web from a command-line interface. With twill, you can navigate through Web sites that use forms, cookies, and most standard Web features.

Comment 1 Till Maas 2007-08-18 09:28:04 UTC
- The Group-tag is wrong:
Group:          TODO

- iirc is python no longer in the default build eviroment, so this package
should not build in devel koji. Afaik you need to add "python-devel" to
BuildRequires.

- Imho you should remove this defination, because they are not used in the spec:
%{!?python_sitearch: %define python_sitearch %(%{__python} -c "from
distutils.sysconfig import get_python_lib; print get_python_lib(1)")}

- This should be imho ended with a "/" to show, that it is directory, also it is
not working, 
%{python_sitelib}/%{name}-%{version}-py%{pyver}.egg-info

so it should be:
%{python_sitelib}/%{name}-%{version}%{beta_ver}-py%{pyver}.egg-info/
(the %{beta_ver} was missing)

- The license, which is in docs/LICENSE.txt in the tarball is not packaged.

- Also the everything else in docs/ is not packaged but may be useful, e.g.
ChangeLog and all the .txt files, and the example in advocacy.

- The Requires seem not to be sufficient, too. E.g. it seems that
pyhon-mechanize should be required and imho python does not need to be in Requires.

- The buildarch should be noarch:
BuildArch:      noarch

Comment 2 Luke Macken 2007-08-18 18:33:28 UTC
Updated RPM:
Spec URL: http://lmacken.fedorapeople.org/rpms/twill.spec
SRPM URL: http://lmacken.fedorapeople.org/rpms/twill-0.9-0.1.b1.fc7.src.rpm

I fixed almost everything that you mentioned, Till.  The only thing that is a
bit questionable is the mechanize requirement.  It looks as if twill ships it's
own mechanize code, so I don't think we need to worry about pulling our own in.
 What do you think?

Comment 3 Till Maas 2007-08-18 19:13:11 UTC
(In reply to comment #2)

> I fixed almost everything that you mentioned, Till.  The only thing that is a
> bit questionable is the mechanize requirement.  It looks as if twill ships it's
> own mechanize code, so I don't think we need to worry about pulling our own in.
>  What do you think?

From the twill hp:
| pyparsing, mechanize, and BeautifulSoup are included with twill for convenience,
| but are under their own licenses.

Fedora contains a newer version of pyparsing, the same version of mechanize and
an older version of clientform (also in devel, so if there is none, imho a bug
should be filed).  I cannot find BeatifulSoup being included in twill and
subproccess is included in python-2.5 .

Do you know how setup.py works? The best approach would be to patch it in a way
that it accepts an argument, e.g. "--exclude-other-packages", that makes the
other packages not being installed and submit this upstream. Otherwise it seems
a simple patch to setup.py is enough to make these packages not being installed.
With these being installed afaik you have to add each license to the License Tag
and also it may violate the Guidelines, because it is imho the same case with
binary packages that contain their own copy of a library.

I also noticed that twill contains some tests, maybe these should be run in
%check, but I do not know how to do this, but I saw someone mentioning this in
another review. But he wrote that there are no tests, so %check is not needed. ;-)

All the changes look good, btw.

Comment 4 Mamoru TASAKA 2007-12-16 05:06:49 UTC
Any progress on here?

Comment 5 Mamoru TASAKA 2007-12-27 16:39:19 UTC
ping again?

Comment 6 Mamoru TASAKA 2008-01-17 14:36:40 UTC
I will close this bug if no response from the reporter is received within
ONE WEEK.

Comment 7 Luke Macken 2008-01-25 18:37:26 UTC
Sorry, I'm still interested in this package.  I will try and polish it up this
weekend.

Comment 8 Luke Macken 2008-01-30 16:05:45 UTC
Ugh, this will be a bit more work than I expected.

    * Moved 'mechanize' to '_mechanize_dist', 'ClientForm' to
    '_mechanize_dist.ClientForm', as requested for Debian dists.

...which means that they forked a bunch of modules (pyparsing, ClientForm,
mechanize).  I'm working on patch to hopefully resolve this (depending on how
bad they forked).

Comment 9 Matthias Saou 2008-03-22 00:17:58 UTC
FWIW :
- Debian has this package called "python-twill", we might want to do the same.
- A plugin of the Elisa media center requires twill, so I'm also interested in
seeing this package getting approved. When you have a new package, I could do
the review if needed.

Comment 10 Jason Tibbitts 2008-07-02 21:07:32 UTC
I'm going to go ahead and mark this as not being ready for review.  Please just
clear the whiteboard if you would like it to go back into the review queue.

Comment 11 Matthias Saou 2008-10-21 17:32:51 UTC
Since I still need the package for elisa, I've updated Luke's 0.9 pre-version to 0.9 final. Note that I'm not really interested in sorting out the issues (if still present) not maintaining the package...

I did rename it to python-twill, though :
http://thias.fedorapeople.org/review/python-twill/

Comment 12 Matthias Saou 2008-12-24 18:43:57 UTC
I've updated the spec and package available at the above address to include a quick patch and requirements in order to avoid using the internal forked python code/projects and use the code already available in Fedora packages instead. Luke : If you could review those changes if you are still interested in being the package maintainer, that would be great (I haven't tested more than a rebuild).

Note also that while looking into this, I've seen that the "mechanize" available in Fedora also/already includes a fork of BeautifulSoup.

Comment 13 Matthias Saou 2009-01-08 18:08:52 UTC
Clearing whiteboard (forgot to do that before!) and assigning to myself for review.

Luke : Could you please confirm that you are still interested in maintaining this package? And check + merge my changes if they look okay to you. I'll then proceed to the formal review.

If you are no longer interested, I'd be willing to become the maintainer as long as someone else is wanting to the review.

Comment 14 Matthias Saou 2009-01-14 14:54:28 UTC
Ping? I'm going to need this package for elisa-plugins-ugly in another repo.

Comment 15 Matthias Saou 2009-02-17 09:57:51 UTC
Removing the fedora-review '?' flag that Jason set, in case that's why this package isn't getting any attention.

Luke, are you still alive? I really think that the package is in good shape now, working fine, and easy to review :
http://thias.fedorapeople.org/review/python-twill/

Comment 16 Matthias Saou 2009-02-28 17:31:19 UTC
Luke, could you give a life sign? It's been over a year since your last comment here :-)

And either confirm that you still want to maintain this package, in which case I'll review it, or that you don't, in which case I'll pick it up and need someone to review it.

Comment 17 Luke Macken 2009-03-04 21:47:03 UTC
Sorry guys, I seemed to have dropped the ball on this one.

I used to have code that relied on this package, but I no longer use it.  Thus, I do not wish to maintain this package.

Comment 18 Jason Tibbitts 2009-03-06 18:01:39 UTC
I'll close it out.

Comment 19 Matthias Saou 2009-03-06 19:34:09 UTC
(In reply to comment #18)
> I'll close it out.  

...meaning I can't pick it up half-way and need to open a new review if I'm now interested in maintaining this package?

Comment 20 Jason Tibbitts 2009-03-06 19:55:59 UTC
Always best if you open a new ticket in any case.  But if you really insist you can just reopen this one.  It's so far back in the list of tickets that there's little chance that anyone will notice, however.

Comment 21 Matthias Saou 2009-04-12 10:21:17 UTC
Just for reference, I've submitted python-twill for review as bug #495357.


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