Bug 466997 - Review Request: sl - Joke command for when you type 'sl' instead of 'ls'
Review Request: sl - Joke command for when you type 'sl' instead of 'ls'
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-14 20:28 EDT by Marc Bradshaw
Modified: 2008-10-28 01:06 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-28 01:06:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
dennis: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marc Bradshaw 2008-10-14 20:28:38 EDT
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-14 23:07:03 EDT
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 04:03:58 EDT
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 04:05:40 EDT
Forgot to say that there is a dot missing at the end of %description.
Comment 4 Marc Bradshaw 2008-10-15 04:38:47 EDT
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 10:24:05 EDT
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 10:29:15 EDT
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 11:15:26 EDT
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 11:33:41 EDT
One more thing:
* %dist tag
  - Please remove %dist tag from %changelog entry.
Comment 9 Mamoru TASAKA 2008-10-22 09:47:22 EDT
ping?
Comment 10 Marc Bradshaw 2008-10-23 07:57:54 EDT
apologies, should be able to get something up this coming weekend.
Comment 11 Marc Bradshaw 2008-10-24 23:26:54 EDT
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 05:19:59 EDT
looks good to me, final word is for Mamoru.
Comment 13 Mamoru TASAKA 2008-10-25 11:05:17 EDT
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 05:44:55 EDT
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 00:19:56 EDT
CVS Done
Comment 16 Marc Bradshaw 2008-10-28 01:04:53 EDT
thanks all, imported and building.

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