[SCM] crtmpserver/master: Correcting release

[Andriy Beregovenko]

Hi Reinhard,

On Sat, Nov 19, 2011 at 11:07:34AM +0100, Reinhard Tartler wrote:
> 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.
Well...  This is your participation as an Debian maintainer. Am I right?
BTW, I do not ask you to do something RIGHT NOW. Your reaction is not clear
to me.
 
> 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.
Ok.

> 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.
Ok, I will comment my next commits more descriptive and more accurately.

> > 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
Done.
 
> > +  * Makes main configuration script more clean
> 
> I notice a number of whitespace issues in the scripts, making them 'unclean'
Definitely!!! Btw that commit is not about whitespaces :)
 
> > +  * 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.
I can't cause that commit removes MANY files, but I need only one(two)
file(s).
 
> 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.
That was error, already fixed.

> 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?
Yes. It is ready to use out-of-box. But there is error in rc script, 
already fixed.
 
> > +  * Make some cosmetic fixes
[ ... ] 
> In general, the changes in debian/changelog should state the differences
> to the previous version that was uploaded to the archive.
Already remade it.

> Having said this, I fear the package still requires work :-(
> 
> -- 
> Gruesse/greetings,
> Reinhard Tartler, KeyID 945348A4

-- 
Best regards,
Andriy
0xBDDBDAE3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20111120/1e3d18ac/attachment-0001.pgp>