Bug 453465 - Review Request: transbot - An irc bot designed to translate between languages on separate channels
Summary: Review Request: transbot - An irc bot designed to translate between languages...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-06-30 19:29 UTC by John McLean
Modified: 2008-08-12 13:29 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-08-12 13:29:11 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description John McLean 2008-06-30 19:29:59 UTC
Spec URL: https://fedorahosted.org/lingobot/browser/transbot.spec
SRPM URL: https://fedorahosted.org/lingobot/browser/transbot-0.1-1.src.rpm
Description: transbot is an irc bot that translates between languages on separate irc channels (connecting at the moment to irc.freenode.net).  Users must specify the language each channel is running.  It takes input from one channel, translates it, and outputs the translated text into other channels in that channel's specified language.  Handles public messages, topic changes, mode changes, join/leave events, and auto-joins to invited channels.

This is my first package, and I'll need a sponsor for it.

Comment 1 manuel wolfshant 2008-07-06 00:08:59 UTC
for a starter;
- ditch the name, version and release macros from the first 5 lines, as they are
identical to the ones used below
- the URL link should point to the place where one can found the project that
you are packaging, not to your [empty] page in fedora's wiki.
- a better %source value would be
https://fedorahosted.org/lingobot/browser/transbot-0.1-1.src.rpm?format=raw
- the changelog must contain at least one entry

Comment 2 John McLean 2008-07-08 00:50:58 UTC
-the macros in the first 5 lines are gone.
-the url link now points to http://git.fedorahosted.org/git/lingobot.git
-the %source you suggested won't work properly because of the '?format=raw'
tagged onto the end.  The spec file searches for a file called
'transbot-0.1-1.src.rpm?format=raw'.  Therefore, I've simply hosted the project
on another site that does have a direct link to the source tarball. It should
work fine.
-The changelog now has an entry.

Comment 3 Mamoru TASAKA 2008-07-31 14:11:06 UTC
Please post the new URLs of your spec/srpm when you modify them. Please make it sure
that you change the release number of your srpm every time you modify your spec/srpm
(if the version number does not change).

Comment 4 John McLean 2008-07-31 15:25:21 UTC
I just fixed a small bug in the program itself.  I've incremented the release
number and uploaded the new srpm and spec. 

The spec url is the same.
SRPM: http://jesusfreak91.googlepages.com/transbot-0.1-2.fc9.src.rpm

Comment 5 John McLean 2008-08-01 13:57:20 UTC
Added utf-8 character support. 

new srpm url: http://jesusfreak91.googlepages.com/transbot-0.1-3.fc9.src.rpm

Comment 6 Mamoru TASAKA 2008-08-03 16:03:44 UTC
For 1.0-3:

* License
  - (As you seem to be the upstream developer) unless you mention
    in the tarball that this package is licensed under GPLv2+,
    we regard the license of this package to be GPL+ (from
    section 9)

* Source
  - The tarball which can be downloaded from the URL differs from
    the one in the tarball.
    When you modify the tarball itself, please change the version number
    and upload the new one.

* %defattr
  - We now recommend %defattr(-,root,root,-)

* General rpmlint issue
--------------------------------------------------------------
transbot.noarch: E: non-executable-script /usr/lib/python2.5/site-packages/trans/transbot.py 0644
transbot.noarch: E: standard-dir-owned-by-package /usr/share/man/man1
--------------------------------------------------------------
  - If the permission of transbot.py is correct (as 0644), then
    this script should not have shebang.

  - This package must not own /usr/share/man/man1 (owned by filesystem)

Comment 7 John McLean 2008-08-04 14:00:10 UTC
-I've fixed the licensing issues.  It is now simply GPL+

-The latest source tarball has been uploaded at the same url.  I simply forgot to upload it last time I made changes.  I'm still forming habits concerning uploading changes and bumping the release numbers.

-%defattr is fixed.

-The rpmlint issues should be cleared up.  The shebang was left over from some testing.  The package shouldn't own /usr/share/man/man1 anymore.

Comment 8 John McLean 2008-08-04 14:02:51 UTC
forgot to add this to my last comment:

new srpm: http://jesusfreak91.googlepages.com/transbot-0.1-4.fc9.src.rpm

Comment 9 Mamoru TASAKA 2008-08-05 18:30:20 UTC
Well,

* Currently I don't know how to use this, however when I just type $ transbot,
  I get a traceback as
---------------------------------------------------------------------------
$ transbot 
Traceback (most recent call last):
  File "/usr/bin/transbot", line 37, in <module>
    channel_list = conf_file.readline().split ( ':' ).lower() [ 1 ]
AttributeError: 'list' object has no attribute 'lower'
----------------------------------------------------------------------------
   Is this expected?

(In reply to comment #7)
> -The latest source tarball has been uploaded at the same url.  I simply forgot
> to upload it last time I made changes.  I'm still forming habits concerning
> uploading changes and bumping the release numbers.

* What I asked you is to change the _version_ number when you modify the tarball
  itself (release number is rpm specific). 

  Changing tarball without changing version may confuse people who are trying to 
  use this package.

Comment 10 John McLean 2008-08-05 19:13:14 UTC
(In reply to comment #9)
> Well,
> 
> * Currently I don't know how to use this, however when I just type $ transbot,
>   I get a traceback as
> ---------------------------------------------------------------------------
> $ transbot 
> Traceback (most recent call last):
>   File "/usr/bin/transbot", line 37, in <module>
>     channel_list = conf_file.readline().split ( ':' ).lower() [ 1 ]
> AttributeError: 'list' object has no attribute 'lower'
> ----------------------------------------------------------------------------
>    Is this expected?
> 
The traceback came from a typo.  I'll need to go back and fix that. 

> (In reply to comment #7)
> > -The latest source tarball has been uploaded at the same url.  I simply forgot
> > to upload it last time I made changes.  I'm still forming habits concerning
> > uploading changes and bumping the release numbers.
> 
> * What I asked you is to change the _version_ number when you modify the
> tarball
>   itself (release number is rpm specific). 
> 
>   Changing tarball without changing version may confuse people who are trying
> to 
>   use this package.

Alright.  I guess I read your comment too quickly and equated version with release in my mind.  My apologies.  For future reference, should the version number be changed even for one-line corrections?

Comment 11 John McLean 2008-08-05 19:48:23 UTC
uploaded new srpm:

http://jesusfreak91.googlepages.com/transbot-0.2-1.fc9.src.rpm

Comment 12 Mamoru TASAKA 2008-08-06 07:16:58 UTC
(In reply to comment #10)
> > * What I asked you is to change the _version_ number when you modify the
> > tarball
> >   itself (release number is rpm specific). 
> > 
> >   Changing tarball without changing version may confuse people who are trying
> > to 
> >   use this package.
> 
> Alright.  I guess I read your comment too quickly and equated version with
> release in my mind.  My apologies.  For future reference, should the version
> number be changed even for one-line corrections?

If you want to modify the tarball itself and wants to publish a new tarball
_formally_, please.
(Note: some upstream developer and also Fedora package submitter doesn't publish
       new tarball formally until the review request he/she submitted is passed)

Well,
* Now this package itself is okay
* You are currently in need of sponsor.
  In this case you have to either submit another review request or do pre-review
  of other person's review request as described in
  http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
  You have 2 other review requests, which seem good to some extent (they
  need fixing on some points... however it is not sure if I have enough time
  to review your other review requests at the moment)

--------------------------------------------------------------------------------------
                      This package (transbot) is APPROVED by mtasaka
--------------------------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor. At the stage, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 8/9, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Comment 13 John McLean 2008-08-06 14:05:20 UTC
I've requested sponsorship.  My FAS name is mcleanj.

Comment 14 Mamoru TASAKA 2008-08-06 14:16:54 UTC
Confirmed. Now I am sponsoring you. Please follow "Join" wiki again.

Removing NEEDSPONSOR.

Comment 15 John McLean 2008-08-06 21:47:57 UTC
New Package CVS Request
=======================
Package Name:  transbot
Short Description: IRC translation bot
Owners: mcleanj
Branches: F-8 F-9
InitialCC: mcleanj
Cvsextras Commits: yes

Comment 16 Kevin Fenzi 2008-08-07 21:07:17 UTC
cvs done.

Comment 17 Mamoru TASAKA 2008-08-08 18:22:09 UTC
Please make it sure that the EVR (Epoch-Version-Release) of rpm has the order
devel >= F-9 >= F-8

Currently F-8 transbot has transbot-0.2-3.fc8 so rawhide (F-10) transbot
must have higher EVR than 0.2-3.fc8. Please bump the release number of
F-10/9 rpms.

Note:
When you want to fix F-9/8 rpm while you don't have to modify F-10 rpm, you
can use "Release: 1%{?dist}.1" for example because 0.2-1.fc10 > 0.2-1.fc8.1 .


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