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

Bug#851756: RFS: telegram-desktop/1.0.0-2



On Wed, Jan 18, 2017 at 10:07:34PM +0300, Коля Гурьев wrote:
> I've putted the current changes back to [1].
> 
> I would appreciate your advice about my debian/rules. What can I improve it?
> 
> [1] https://mentors.debian.net/debian/pool/contrib/t/telegram-desktop/telegram-desktop_1.0.0-1.dsc

It seems this one is using a different .orig.tar.gz than the first one.
Neither of them being the .tar.gz I download from github.  bad.

Then I looked at that git repo you put in Vcs-*.  I'm not sure about the
format used there, but I'd say to get this release upload and then sync
up with whatever's there.  (or have me understand what's going on
there).


Then:

* d/compat:
  + please use 10.  Read the relevant part of debhelper(7) before.
* d/control:
  + as said below, I'd say this is good to go in main
  + Build-dep on 'gyp:all', why that architecture qualification?  I'm
    not even sure what that is supposed to mean
  + you build-dep on libss1.0-dev, doesn't this work with libss1.1?  If
    it doesn't please open an upstream issue, and be ready to have a
    debian bug as soon as it's accepted into the debian archive
* d/manpage.1.ex:
  + this is an example empty file, get rid of it (or even better, write
    a useful one)
* d/README.Debian:
  + there is a todo there.  I'm pretty sure users are not interested in
    knowing you need to "Sort out dpkg-shlibdeps warnings".  Please move
    that list elsewhere (this could either be a d/TODO file, or bugs in
    some bug tracker (like the Debian one, once it's accpeted)
* d/shlibs.local:
  + why?
* d/telegramdesktop.desktop:
  + upstream already ships one, why duplicate?
* d/telegram-desktop.doc-base.EX:
  + this is an example file, get rid of it
* d/p/Avoid-depending-on-static-libraries.patch:
  + it links unrelated bugs, or why does it anyway?
  + can you work on a patch that does that by means of a compile flag,
    and send it upstream?
* d/p/Fix-autorestart-when-new-language-is-selected.patch:
  + this definitely should be upstream already
* d/rules:
  + BUILD_MODE doesn't seem to actually change anything except for the
    name of a directory.  if so I guess you can just get rid of that
  + please drop 'nostrip' handling; if you strip binaries, then dh_strip
    won't have anything left to strip, and the genereted -dbgsym package
    won't contain anything useful
  + also drop all of those INSTALL_* things.  Just call dh_install to
    copy the binary in place (no need to create the directory with it)
    and then mv(1) to rename it.  same for the icon: mkdir + cp is cool
    (but feel free to keep using install(1) for this one if you like)
    Personally I'd even prefer dh-exec over that, because I like a more
    declarative packaging, but you're call.
  + I think you could just add a `--buildsystem cmake` in the dh call,
    then the override_dh_auto_build could go, as could the "configure
    with cmake" be substituted by a dh_auto_configure call.  OTOH this
    requires changing other bits of how you're building the package.
  + also the parallel stuff should really be handled by dh itself,
    instead of make.  besides, just doing the makes very little of the
    build parallel.
* the ITP bug is named differently than the RFS and the package itself.
  please pick a name and uniform all the things.


Be aware that I don't know gyp, and really the structure of the upstream
package is very weird to me.
Also I've yet to look at the upstream package itself, and doing a
license check.

> By the way, this is my nickname in Telegram: https://t.me/mymedia

Great, I'll keep it in mind if at some point I'll need to contact you
(I'm @mattia).
That said I'd prefer if at least this one sponsorship request is done
in public.

> > Why is it in contrib?
> 
> From what I understand, there are packages with dependencies on non-free
> software in contrib section. This program does not work if Telegram server
> is stopped for any reason. But this server is proprietary, more accurately,
> it does not even distribute in public. So I thought the package has to be in
> contrib.  Correct me, please, if I am wrong.

Indeed, it should be in main.

-- 
regards,
                        Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540      .''`.
more about me:  https://mapreri.org                             : :'  :
Launchpad user: https://launchpad.net/~mapreri                  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-

Attachment: signature.asc
Description: PGP signature


Reply to: