guix-patches
[Top][All Lists]
Advanced

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

[bug#45721] Telegram Desktop (v9)


From: Leo Prikler
Subject: [bug#45721] Telegram Desktop (v9)
Date: Sun, 17 Jan 2021 01:36:37 +0100
User-agent: Evolution 3.34.2

Hi Raghav,

Am Samstag, den 16.01.2021, 19:19 -0500 schrieb Raghav Gururajan:
> 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.
Nvm then.

> > 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.
Again, you can (define gpl2+ license:gpl2+) at the start of the module,
so that existing definitions can be kept the same, but your packages
adhere to the license: style.

> > LGTM.
> 
> Cool!
> 
> > Is there a less broad way of doing this?  E.g. replacing c++-17 by
> > c++-
> > 11?
> 
> Updated in v10.
Yes, okay.

> > 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.
Already discussed that one in IRC.

> > 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.
Okay.

> > 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.
Thanks.

> > It appears, that this library carries a few more (free) licenses
> > with
> > it.  Perhaps this needs to be revised?
> 
> Updated in v10.
Seems okay, I also saw you double-checking in IRC.

> > 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.
Use full commit hashes please.

> > 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.
Perhaps, but it seems kinda weird, that this package gets special
treatment in how we accept anything else it might pull in.

> > 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 don't particularly agree with the way this has been solved in v10. 
Do we really need to keep around custom forks and old versions?  Can we
not instead patch tg_owt?

> > 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.
Yep, that sounds better.

> > 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.
Fair enough.

> > 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.
That would make sense in my opinion.

Regards,
Leo






reply via email to

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