[SCM] crtmpserver/master: Correcting release

[Reinhard Tartler]

Hi,

Sorry for being late, but I am currently increadibly busy with my
day-job and have a numer of other open source projects that compete with
my available time.  I've now spend over 30 minutes reviewing your
package, which is much longer than I intended to invest. Unfortunately,
I also have some comments.

First of all, I noticed that you've pushed the tag for this upload to
the repository. Please don't, leave that to the uploader. In this
particular case, I think we need to do some additional changes so that
the tag will not represent what will eventually go into the archive.

On Fr, Nov 11, 2011 at 17:31:00 (CET), jet-guest at users.alioth.debian.org wrote:

> The following commit has been merged in the master branch:
> commit 1ba6610128bcf372e3295496d9f933470a3bed89
> Author: Andriy Beregovenko <jet at jet.kiev.ua>
> Date:   Fri Nov 11 18:23:54 2011 +0200
>
>     Correcting release

This commit message does not really match the contents. I notice this
applies to a number your commits, which makes doesn't make reviewing any
easier.

> diff --git a/debian/changelog b/debian/changelog
> index 12d651d..7162b0d 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,29 @@
> +crtmpserver (0.0~dfsg+svn611.1-1) UNRELEASED; urgency=low

please merge the "UNRELEASED" entry of '0.0~dfsg+svn611-1' with this one

> +
> +  [ Reinhard Tartler ]
> +  * Refresh patches.
> +
> +  [ Andriy Beregovenko ]
> +  * Add basic Lua configuration scripts
> +  * Returns back package "crtmpserver"

I fail to understand this. Remember, that debian/changelog is exposed to
the users of your package, for example by apt-listchanges and others.

> +  * Add more application conf scripts

Which ones?

> +  * Makes main configuration script more clean

I notice a number of whitespace issues in the scripts, making them 'unclean'

> +  * Add rc script

I think we discussed this earlier this year. In commit
http://anonscm.debian.org/gitweb/?p=pkg-multimedia/crtmpserver.git;a=commitdiff;h=e4f2bce15ab888aabcb97c035a09373bbf44c5af,
I have removed it, and in
http://anonscm.debian.org/gitweb/?p=pkg-multimedia/crtmpserver.git;a=commitdiff;h=c06ba11fbb9741c8de6253a64e04e9d978b8346a
you are adding it back with 'Add rc script'. 

I'm still not convinced that this is the way to go, but if you insist
on this, you should better have used 'git revert', so that this fact
becomes clear. It would have saved my a lot of time, BTW.

Moreover, I'm not happy with the script, because in order to use it,
users *have* to edit it. The previous solution with
/etc/default/$servername is much more common in debian packages. Still,
defaulting it to 'no' is something I personally feel very uncomfortable
with.

I'm uncomfortable with it because IMO, installing a service should
instantly make it usable. For instance, installing the apache package
starts a webserver that serves a basic 'welcome' webpage. Can't we do
something similar for crtmpserver?

> +  * Make some cosmetic fixes

Unclar. Clarify or drop it.

> +  * Update installed list of files for package crtmpserver
> +  * Add installing config scripts for all applications
> +  * Rename application scripts to make future updates more easy
> +  * Add portinstall script for crtmpserver-apps package

typo

> +  * Rename scripts back :)

Uh?

> +  * Install application cofngis as example, and while postinstall stage "configure" copy it to etc if not exists already.

typo (I know that some of them have been fixed in later commits)

> +  * Move enabled_applications.conf to -apps package
> +  * Move enabled_applications.conf to -apps package
> +  * Add  to packages
> +  * Update gbp.conf to ignore .svn while import sources
> +  * Imported Upstream version 0.0~dfsg+svn611.1
> +
> + -- Andriy Beregovenko <jet at jet.kiev.ua>  Fri, 11 Nov 2011 18:23:04 +0200
> +

In general, the changes in debian/changelog should state the differences
to the previous version that was uploaded to the archive.

Having said this, I fear the package still requires work :-(

-- 
Gruesse/greetings,
Reinhard Tartler, KeyID 945348A4