guix-patches
[Top][All Lists]
Advanced

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

[bug#45721] Telegram Desktop (v9)


From: Raghav Gururajan
Subject: [bug#45721] Telegram Desktop (v9)
Date: Sat, 16 Jan 2021 19:19:01 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Icedove/78.6.0

Hi Leo!

congratulations on getting a working Telegram Desktop.  I haven't yet
built this version on my own, but I want to comment on the patches a
little.

Thanks! Couldn't have done without your help. :-)

LGTM, but there seem to be whitespace issues.  Any idea?

Those are from the patch file taken from upstream. The pack-def is clean.

Personal nitpick, but those should likely be prefixed with license:
To keep backwards-compatibility, you can (define gpl2+ license:gpl2+)
inside the module.

Hmm. I think we have to make changes to all pack-def in this module. As the licenses doesn't use prefix. Can be a separate task.

LGTM.

Cool!

Is there a less broad way of doing this?  E.g. replacing c++-17 by c++-
11?

Updated in v10.

FWIW there seems to also exist an older version over at
https://gitlab.com/nimf-i18n/nimf
Would it be worth packaging that?

Hmm, not sure. I can package it later though.

In my opinion the synopsis should be "Lightweight input method
framework" and the description should mention, that this specific fork
has a special focus on Korean.

Updated in v10.

LGTM, albeit admittedly weird.

Haha, yeah.

It's not "the", just "a".  Usually such projects describe Material
Design in some way, but apparently it has come to a point where just
throwing around the word "Material" is enough.

Updated in v10.

I find the following more useful:
[range-v3 is] an extension of the Standard Template Library that
makes its iterators and algorithms more powerful by making them
composable.  Unlike other range-like solutions which, seek to do away
with iterators, in range-v3 ranges are an abstration layer on top of
iterators.

Updated in v10.

It appears, that this library carries a few more (free) licenses with
it.  Perhaps this needs to be revised?

Updated in v10.

LGTM.

Cool!

LGTM.

Cool!

Use a proper version.  Packages, that build directly from git without
any tagged versions usually have a preamble of
   (let ((commit <hash>)
         (revision <number))
     (package ...))

Updated in v10.

Is there a way of making this checkout non-recursive?  I know you've
made that change in accordance to an upstream recommendation, but one
ought to look at it a little closer.

Not sure, but we need the sub-modules any way for build.

It seems that some of those inputs are also found as third_party/
libraries.  Can you remove their respective sources from the package?

Updated in v10.

I really don't like that synopsis and description.  Granted, upstream
offers little to work with, but there ought to be a better way of
phrasing this.

Updated in v10.

By the way, it would appear we already have some WebRTC stuff packaged,
but no direct "webrtc" package (which I guess is normal, given that it
is a protocol and not an individual piece of software).  How tightly is
Telegram coupled to this specific implementation?  Would there be a way
of replacing it with something else (kinda like our udev/eudev
situation)?

Nah, telegram made a hard fork. There are some telegram-specific classes and objects.

Would there be a way of moving this into another module, e.g. (gnu
packages messaging)?

I first tried there, but the was circular dependency. So moved it to separate module. We can also move telegram-related stuff like tg_owt etc, to this module in the future.

Thank you so much for reviewing. :-)

Regards,
RG.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]