Bug 466997 - Review Request: sl - Joke command for when you type 'sl' instead of 'ls'
Summary: Review Request: sl - Joke command for when you type 'sl' instead of 'ls'
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-10-15 00:28 UTC by Marc Bradshaw
Modified: 2008-10-28 05:06 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-28 05:06:21 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Marc Bradshaw 2008-10-15 00:28:38 UTC
Spec URL: http://marcbradshaw.co.uk/packages/review/sl/sl.spec
SRPM URL: http://marcbradshaw.co.uk/packages/review/sl/sl-3.03-3.fc9.src.rpm

Description:
The sl (Steam Locomotive) command is a joke which displays a train on your
terminal when you accidentally type 'sl' instead of 'ls'


The sl package has been part of the dribble repo for some time, during review for the rpmfusion repo the licence issues were resolved[1] and the package should now be fine for inclusion in fedora.

1 - http://lists.rpmfusion.org/pipermail/rpmfusion-developers/2008-October/001599.html

Comment 1 Mamoru TASAKA 2008-10-15 03:07:03 UTC
I am happy to see this package on Fedora, because this is well-known to
Japanese Linux users ;)

Comment 2 Patrice Dumas 2008-10-15 08:03:58 UTC
The mail transcript should be directly in the package as asource, so

Source2: http://marcbradshaw.co.uk/packages/review/sl/sl-license-mail.txt

would work, with cp and adding to %doc.

I suggest removing the /bin/ and let the commands be searched on the path.

You should use cp -p and install -p (for the manpage) to keep timestamps.

Also I suggest doing something along:

iconv -f iso-2022-jp README -t utf8 > README.conv README.conv && \
 touch -c -r README && \
 mv README.conv README

Also the -f of rm is not needed, it is always the default in rpm.

Comment 3 Patrice Dumas 2008-10-15 08:05:40 UTC
Forgot to say that there is a dot missing at the end of %description.

Comment 4 Marc Bradshaw 2008-10-15 08:38:47 UTC
Thanks Pat,

I assume you mean mv -f rather than rm -f and have incorporated the suggestions into a new revision.

http://marcbradshaw.co.uk/packages/review/sl/sl-3.03-4.fc9.src.rpm

Comment 5 Patrice Dumas 2008-10-15 14:24:05 UTC
The timestamp of the source archive is not kept:

-rw-rw-r-- 1 dumas dumas 20480 juil. 22  1999 sl.tar
-rw-rw-r-- 1 dumas dumas 20480 févr. 13  2008 ../SOURCES/sl.tar

Otherwise seems ok to me.

Comment 6 Patrice Dumas 2008-10-15 14:29:15 UTC
Forgot one thing. In general I think that it is pretty bad to use
a two letter command, since the number of two letter command names
is scarce and they should be used wisely. However in that case I think
that the name is not really taken since sl could just go if something 
serious wants the command name.

Comment 7 Mamoru TASAKA 2008-10-15 15:15:26 UTC
Some notes for -4:

* License
  - Please write explicitly from which you borrowed the license
    text in the spec file (and also in sl.COPYRIGHT) as comments.
    Perhaps it is from:
    http://ftp.debian.org/debian/pool/main/s/sl/sl_3.03-15.diff.gz

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

* manfile
  - Files under %_mandir are automatically marked as %doc.

Comment 8 Mamoru TASAKA 2008-10-15 15:33:41 UTC
One more thing:
* %dist tag
  - Please remove %dist tag from %changelog entry.

Comment 9 Mamoru TASAKA 2008-10-22 13:47:22 UTC
ping?

Comment 10 Marc Bradshaw 2008-10-23 11:57:54 UTC
apologies, should be able to get something up this coming weekend.

Comment 11 Marc Bradshaw 2008-10-25 03:26:54 UTC
Sorry for the delay, been a busy week.  The changes have been made to new SRPM located at...

http://marcbradshaw.co.uk/packages/review/sl/sl-3.03-5.fc9.src.rpm

Comment 12 Patrice Dumas 2008-10-25 09:19:59 UTC
looks good to me, final word is for Mamoru.

Comment 13 Mamoru TASAKA 2008-10-25 15:05:17 UTC
Oops, one note:
* Man file
  - Please move man.1 to %_mandir/ja/man1.
    (Maybe %lang(ja) is preferable).

                             ( This package (sl) is APPROVED by mtasaka ) 
                 (@@@)
             (    )
          (@@@@)

        (   )
      ++      +------ ____                 ____________________ 
      ||      |+-+ |  |   \@@@@@@@@@@@     |  ___ ___ Help!__ | 
    /---------|(O) |  |    \@@@@@@@@@@@@@_ |  (O) |_| \O/ |_| | 
   + ========  +-+ |  |                  | |__________________|
  _|--/~\------/~\-+  |__________________| |__________________|
 //// \O========O/       (O)       (O)        (O)        (O)

Comment 14 Marc Bradshaw 2008-10-26 09:44:55 UTC
Thankyou Mamoru.

New Package CVS Request
=======================
Package Name: sl
Short Description: Joke command for when you type 'sl' instead of 'ls'
Owners: deebs
Branches: F-8 F-9 EL-4 EL-5
InitialCC:

Comment 15 Dennis Gilmore 2008-10-27 04:19:56 UTC
CVS Done

Comment 16 Marc Bradshaw 2008-10-28 05:04:53 UTC
thanks all, imported and building.


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