[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [RFR] templates://mserv/{mserv.templates}



Christian Perrier wrote:
> Your review should be sent as an answer to this mail.

This isn't quite a complete review.

>  Template: mserv/mp3_location
[...]
> +_Description: Path to the root of the MP3 archive:
> + Mserv needs to know where MP3 files are located so that it can index
> + them. The files don't need to be arranged in any special way.

Bare "MP3 files" here feels odd.  We don't want to say "your" MP3
files, but how about "its" MP3 files?

Hang on, though - what if (as in fact is the case) my music files
aren't in MP3 format?  The package description claims it supports
Ogg Vorbis format too... and yes, my trial install (on Etch) shows
up a variety of other annoyances, but since it supports music123 I'm
not having any problem getting it to play WAV, Flac, Musepack, MP4
or Speex files either...

   _Description: Path to the root of mserv's music archive:
    Mserv needs to know where its music files are located so that it can
    index them. The files don't need to be arranged in any special way.

> + If no MP3 files are available right now, just enter any directory.
> + In that case,
> + mserv will not be able to play music. To index new MP3 files in the
> + future, you can
> + run "dpkg-reconfigure mserv" to instruct the program about their location.

Avoid the alternative interpretation of "enter [the] directory".
And "instruct about" seems didactic, so say "inform of".

    If no music files are available right now, just enter any directory name.
    In that case, mserv will not be able to play music. To index new music
    files in the future, you can run "dpkg-reconfigure mserv" to inform the
    program of their location.
  
>  Template: mserv/path_invalid
>  Type: boolean
>  Default: false
> -_Description: This path does not exist. Would you like to use it anyway?
> +_Description: Use nonexisting directory anyway?

"Nonexistent" or (rarer, but maybe justified here) "non-existing".

Except that I got this error when it _did_ exist.  Mserv does its
scan as an unprivileged user - it should say something like: 

  _Description: Really use unreadable location?
   Mserv cannot access the directory path you specified. Please choose whether
   you want to use that path anyway.

>  Template: mserv/confirm_update
> +_Description: Really change the MP3 files archive location?

s/MP3 files/music/g, simplifying the noun-pile.

>  Package: mserv
[...]
>  Recommends: mpg321 | mpg123 | vorbis-tools | music123 | sox         

I'd suggest changing that to "mp3-decoder | music123 | sox", but I'll
leave it as it is.

> -Description: local centralised multiuser music server 
> - Mserv is a music server designed to do a number of things better than most
> - systems designed to play mp3s or oggs:
> +Description: centralised multiuser music server - server
                        z

> Standard form for multi-binary packages : one fix part and one package-specific

If we're doing "suite description - package role" we can avoid
"server - server": 

   Description: centralized multiuser music environment - server

> + Mserv is a network music server with the following features:
> 
> Less "advertisment-style". You don't know if it's better than others
> and anyway, package descriptions are not meant for this..:-)

Agreed, but you've dropped the qualifier "local" from the short
description - I wouldn't suggest running it over the Internet.
I'd also suggest that people might want it to turn up in searches on
the word "jukebox".  So bulking it back out a bit:

    Mserv is a local network jukebox server. It features:

> +  - Support for any type of client using a standard TCP protocol;

What does this even mean?  (Does it support SSH clients?)  I can
only imagine it's hinting at the fact (advertised in README.Debian)
that you could connect to it from a MUD... 

> +  - storage of information on MP3 (bitrate, duration, name, author, genre, date
> +    produced, last play date) in an on-disk database;

This has a better candidate lead noun, the "database".  Not that
it's quite what people normally mean when they refer to an "on-disk
database" - it doesn't mean "stored in a .db file instead of in
memory", it means "made of lots and lots of little text files, which
you'll probably have to edit by hand".

> +  - rating information supplied by the user (awful, bad, neutral, 
> +    good, superb);

These go in the same "database" system, so merge the items:

     - text databases for track information (author, name, year, genre,
       last play date, duration) and user ratings (awful, bad, neutral,
       good, superb);

(Ordering them as they appear in the file; no sign of "bitrate".)

> +  - queuing system (track, album, random album, etc);
                                                      ^.
> +  - random play mode that uses users' ratings;

Is there an easy way of avoiding "uses user"?  How about:

     - random play mode taking users' ratings into account;

> +  - search facilities, status information, statistics, etc. 
                                                              ^;
> +  - user management facilities, four levels of users, encrypted passwords;

(The passwords may be stored encrypted, but they're used as
command-line arguments, and the default admin password is "root"!)

> +  - talker style communication (say, emote etc.);
> +  - play, next, pause, stop, repeat, volume, bass, treble settings;
> +  - on-line and off-line track information editing;
> +  - advanced filter facilities (e.g. 'john=superb', '!good', 'year>1990',
> +    'duration<180', 'genre=pop', 'john=good|fred=unheard' etc.);
                                                            ^^^^^
No need for both "e.g." and "etc." on the same list.

> +  - built-in telnet client (see manual);

There's no _client_ built into the _server_ package.  And in fact
"http://manpages.debian.net/cgi-bin/man.cgi?query=mservcmd"; says
"This is not for interactively talking to a server, for that you 
require a telnet client."  It doesn't even use port 23 - all these
references to telnet are based purely on the fact that it uses a
"telnet-like" remote CLI, which it has already mentioned once (in
that obscure first item) and refers to again below.

Merge all these lines into one at the top saying

     - telnet-style control interface, remotely accessible from any
       appropriate client;

> +  - library interface without need to write TCP code;

Huh?  Interfaces never need to write code - they've got developers
to do that for them.  If this is a feature, it's a feature of the
-dev package (which gets zero "votes" in popcon, by the way).

> +  - command line shell client (in the mserv-client package);

A similar case (popcon: eight votes).

> +  - web client (in the mserv-cgi package);

Ditto (and zero votes).  Shift all these three to the end, or indeed
a see-also paragraph:

    The packages mserv-cgi and mserv-client provide web-based and command-line
    interfaces; mserv-dev provides a development library.

> +  - use of an external player to output, with support for mpg321, mpg123,
> +    ogg123 and freeamp, allowing to broadcast the output or
> +    support other players;

You can't allow, you can only allow particular things - disallowed!
Besides, was it guaranteeing that you can do this, or was it merely
a coming-real-soon-now feature?

Freeamp became Zinf and then died; mpg321 is a relic free version of
a now-equally-free original.  It's possible you might be able to get
mserv to work via xmms2... but I can't imagine why you'd bother.

     - output via an external player, with support for mpg123, ogg123, etc.;

> +  - setuid wrapper for mpg123-compatible players that can 
> +    increase the nice level for low-capability processors.

And when it says "low", wow... I thought _my_ computers were
pitiful.  But if we strip "comes with a" we should probably add a
modifier to clarify that setuid isn't the default:

     - setuid wrapper provided for mpg123-compatible players, which can
       increase the nice level for low-capability processors.
 
> I revamped the whole list to use a noun-based enumeration instead of a
> mix of noun and verb. Using a verb-based enumeration quickly leads to
> repeat the same verb over and over.

(Which is okay if you can then factor out the verb into the intro,
but yes, interesting point.)

Oh.  Now I come to generate my diff I see that we're working against
the Lenny version of mserv, "Maintainer: Nick Estes".  Sid has a
newer version with "Maintainer: Debian QA Group" and different
Build-Depends and Standards-Version.  I'll just send my modified
Lenny versions for now and leave out the patch.
-- 
JBR	with qualifications in linguistics, experience as a Debian
	sysadmin, and probably no clue about this particular package
Template: mserv/mp3_location
Type: string
_Description: Path to the root of the music archive:
 Mserv needs to know where its music files are located so that it can
 index them. The files don't need to be arranged in any special way.
 .
 If no music files are available right now, just enter any directory name.
 In that case, mserv will not be able to play music. To index new music
 files in the future, you can run "dpkg-reconfigure mserv" to inform the
 program of their location.

Template: mserv/path_invalid
Type: boolean
Default: false
_Description: Really use unreadable directory?
 Mserv cannot access the directory path you specified. Please choose whether
 you want to use that path anyway.

Template: mserv/confirm_update
Type: boolean
Default: false
_Description: Really change the music archive location?
 By reconfiguring the location of the music archive when a
 trackinfo database already exists, you risk losing the track info
 database (ratings, last play times etc.).
Source: mserv
Section: sound
Priority: optional
Maintainer: Nick Estes <debian@nickstoys.com>
Build-Depends: sharutils, debhelper (>= 4.1.51), cdbs
Standards-Version: 3.6.1
Homepage: http://www.mserv.org 

Package: mserv
Architecture: any
Depends: adduser, debconf | debconf-2.0, ${shlibs:Depends}, ${perl:Depends}
Recommends: mpg321 | mpg123 | vorbis-tools | music123 | sox
Description: centralized multiuser music environment - server
 Mserv is a local network jukebox server. It features:
 .
  - telnet-style control interface, remotely accessible from any
    appropriate client;
  - text databases for track information (author, name, year, genre,
    last play date, duration) and user ratings (awful, bad, neutral,
    good, superb);
  - queuing system (track, album, random album, etc.);
  - random play mode taking users' ratings into account;
  - search facilities, status information, statistics, etc.;
  - user management facilities, four levels of users, encrypted passwords;
  - talker style communication (say, emote etc.);
  - play, next, pause, stop, repeat, volume, bass, treble settings;
  - on-line and off-line track information editing;
  - advanced filter facilities (e.g. 'john=superb', '!good', 'year>1990',
    'duration<180', 'genre=pop', 'john=good|fred=unheard');
  - output via an external player, with support for mpg123, ogg123, etc.;
  - setuid wrapper provided for mpg123-compatible players, which can
    increase the nice level for low-capability processors.
 .
 The packages mserv-cgi and mserv-client provide web-based and command-line
 interfaces; mserv-dev provides a development library.

Package: mserv-client
Architecture: any
Depends: ${shlibs:Depends}
Description: centralized multiuser music environment - command-line client
 Mserv is a local network jukebox server.
 .
 This package contains a client protocol library and very simple command-line
 client for the mserv server.

Package: mserv-cgi
Architecture: all
Depends: mserv-client, apache | httpd, ${perl:Depends}
Description: centralized multiuser music environment - CGI scripts
 Mserv is a local network jukebox server.
 .
 This package contains CGI scripts that can be used to access and control mserv
 from a web browser.

Package: mserv-dev
Section: devel
Architecture: any
Depends: mserv-client (= ${binary:Version})
Description: centralized multiuser music environment - development files
 Mserv is a network music server.
 .
 This package contains the header files and static libraries needed
 for development.

Reply to: